Skip to content

Commit

Permalink
Handle accumulated affiliations efficiently
Browse files Browse the repository at this point in the history
Affiliations where stored in the mongoose_acc for atomicity, and
performance. If a message processing pipeline in a room starts with
certain affiliations at the time of that message, we don't want to have
some handlers in the pipeline process the message with certain affs,
only to have the affiliations changed concurrently and the next handler,
for the same message, be processed with different ones.

But the code as currently written was quite redundant. The main call to
get the affiliations from the accumulator, was running the hook that
would check the acc, the cache, and the DB. We can skip that hook if
we're already in the context of muc_light and simply check if the
affiliations are already in the accumulator, and only if not then we run
the hook.

Also take the opportunity to tag the affiliations stored in the
accumulator, with the room's jid, just to avoid the (though improbable)
chance of more than one room's affiliations being accumulated under the
same key.
  • Loading branch information
NelsonVides committed Aug 24, 2022
1 parent ba2e32f commit 884ca87
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 23 deletions.
4 changes: 2 additions & 2 deletions src/mod_muc.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1288,13 +1288,13 @@ remove_domain(Acc, HostType, Domain) ->

-spec acc_room_affiliations(mongoose_acc:t(), jid:jid()) -> mongoose_acc:t().
acc_room_affiliations(Acc1, Room) ->
case mongoose_acc:get(?MODULE, affiliations, {error, not_found}, Acc1) of
case mongoose_acc:get(?MODULE, {affiliations, Room}, {error, not_found}, Acc1) of
{error, _} ->
case mod_muc_room:get_room_users(Room) of
{error, not_found} ->
Acc1;
{ok, _Affs} = Res ->
mongoose_acc:set(?MODULE, affiliations, Res, Acc1)
mongoose_acc:set(?MODULE, {affiliations, Room}, Res, Acc1)
end;
_Affs ->
Acc1
Expand Down
41 changes: 25 additions & 16 deletions src/muc_light/mod_muc_light.erl
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
acc_room_affiliations/2,
room_exists/3,
can_access_identity/4]).
-export([get_room_affiliations_from_acc/1]).
-export([get_acc_room_affiliations/2]).

%% For propEr
-export([apply_rsm/3]).
Expand All @@ -77,6 +77,7 @@

-type muc_server() :: jid:lserver().
-type host_type() :: mongooseim:host_type().
-type versioned_affs() :: {ok, aff_users(), binary()}.

%%====================================================================
%% API
Expand Down Expand Up @@ -149,7 +150,7 @@ try_to_create_room(CreatorJid, RoomJID, #create{raw_config = RawConfig} = Creati
| {error, validation_error() | bad_request | not_allowed | not_exists | item_not_found}.
change_room_config(UserJid, RoomID, MUCLightDomain, ConfigReq, Acc1) ->
RoomJID = jid:make(RoomID, MUCLightDomain, <<>>),
{Acc2, AffUsersRes} = get_room_affiliations_from_acc(Acc1, RoomJID),
{Acc2, AffUsersRes} = get_acc_room_affiliations(Acc1, RoomJID),
case mod_muc_light_room:process_request(UserJid, RoomJID, {set, ConfigReq},
AffUsersRes, Acc2) of
{set, ConfigResp, _} ->
Expand Down Expand Up @@ -516,15 +517,15 @@ can_access_room(_, Acc, Room, User) ->
none =/= get_affiliation(Acc, Room, User).

-spec acc_room_affiliations(mongoose_acc:t(), jid:jid()) -> mongoose_acc:t().
acc_room_affiliations(Acc1, Room) ->
case mongoose_acc:get(?MODULE, affiliations, {error, not_exists}, Acc1) of
acc_room_affiliations(Acc1, RoomJid) ->
case get_room_affs_from_acc(Acc1, RoomJid) of
{error, _} ->
HostType = mongoose_acc:host_type(Acc1),
case mod_muc_light_db_backend:get_aff_users(HostType, jid:to_lus(Room)) of
case mod_muc_light_db_backend:get_aff_users(HostType, jid:to_lus(RoomJid)) of
{error, not_exists} ->
Acc1;
{ok, _Affs, _Version} = Res ->
mongoose_acc:set(?MODULE, affiliations, Res, Acc1)
set_room_affs_from_acc(Acc1, RoomJid, Res)
end;
_Affs ->
Acc1
Expand All @@ -534,16 +535,24 @@ acc_room_affiliations(Acc1, Room) ->
room_exists(_, HostType, RoomJid) ->
mod_muc_light_db_backend:room_exists(HostType, jid:to_lus(RoomJid)).

-spec get_room_affiliations_from_acc(mongoose_acc:t()) ->
{ok, aff_users(), binary()} | {error, not_exists}.
get_room_affiliations_from_acc(Acc) ->
mongoose_acc:get(?MODULE, affiliations, {error, not_exists}, Acc).
-spec get_acc_room_affiliations(mongoose_acc:t(), jid:jid()) ->
{mongoose_acc:t(), versioned_affs() | {error, not_exists}}.
get_acc_room_affiliations(Acc1, RoomJid) ->
case get_room_affs_from_acc(Acc1, RoomJid) of
{error, not_exists} ->
Acc2 = mongoose_hooks:acc_room_affiliations(Acc1, RoomJid),
{Acc2, get_room_affs_from_acc(Acc2, RoomJid)};
Res ->
{Acc1, Res}
end.

-spec get_room_affs_from_acc(mongoose_acc:t(), jid:jid()) -> versioned_affs() | {error, not_exists}.
get_room_affs_from_acc(Acc, RoomJid) ->
mongoose_acc:get(?MODULE, {affiliations, RoomJid}, {error, not_exists}, Acc).

-spec get_room_affiliations_from_acc(mongoose_acc:t(), jid:jid()) ->
{mongoose_acc:t(), {ok, aff_users(), binary()} | {error, not_exists}}.
get_room_affiliations_from_acc(Acc1, RoomJid) ->
Acc2 = mongoose_hooks:acc_room_affiliations(Acc1, RoomJid),
{Acc2, mongoose_acc:get(?MODULE, affiliations, {error, not_exists}, Acc2)}.
-spec set_room_affs_from_acc(mongoose_acc:t(), jid:jid(), versioned_affs()) -> mongoose_acc:t().
set_room_affs_from_acc(Acc, RoomJid, Affs) ->
mongoose_acc:set(?MODULE, {affiliations, RoomJid}, Affs, Acc).

-spec can_access_identity(Acc :: boolean(), HostType :: mongooseim:host_type(),
Room :: jid:jid(), User :: jid:jid()) ->
Expand Down Expand Up @@ -580,7 +589,7 @@ max_occupants(HostType) ->
gen_mod:get_module_opt(HostType, ?MODULE, max_occupants).

get_affiliation(Acc, Room, User) ->
case get_room_affiliations_from_acc(Acc, Room) of
case get_acc_room_affiliations(Acc, Room) of
{_, {ok, AffUsers, _}} ->
case lists:keyfind(jid:to_lus(User), 1, AffUsers) of
{_, Aff} -> Aff;
Expand Down
9 changes: 4 additions & 5 deletions src/muc_light/mod_muc_light_room.erl
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,10 @@

-spec handle_request(From :: jid:jid(), RoomJID :: jid:jid(), OrigPacket :: exml:element(),
Request :: muc_light_packet(), Acc :: mongoose_acc:t()) -> mongoose_acc:t().
handle_request(From, Room, OrigPacket, Request, Acc1) ->
Acc2 = mongoose_hooks:acc_room_affiliations(Acc1, Room),
AffUsersRes = mod_muc_light:get_room_affiliations_from_acc(Acc2),
Response = process_request(From, Room, Request, AffUsersRes, Acc2),
send_response(From, Room, OrigPacket, Response, Acc2).
handle_request(From, RoomJID, OrigPacket, Request, Acc1) ->
{Acc2, AffUsersRes} = mod_muc_light:get_acc_room_affiliations(Acc1, RoomJID),
Response = process_request(From, RoomJID, Request, AffUsersRes, Acc2),
send_response(From, RoomJID, OrigPacket, Response, Acc2).

-spec maybe_forget(Acc :: mongoose_acc:t(),
RoomUS :: jid:simple_bare_jid(),
Expand Down

0 comments on commit 884ca87

Please sign in to comment.