Skip to content

Commit 0dce848

Browse files
author
Loïc Hoguin
authored
CQ: Confirm related optimisations (#7516)
* CQ: Optimise shared store remove when nothing to remove The message(s) was/were already optimised out during write when the client acked faster than we could process the write message. * Optimise sets:subtract in single-confirm case This happens due to message store optimisations that tries to confirm as fast as possible on write or ack, even if that means processing a single confirm. The ack scenario is common because clients tend to not be built for multi-ack. The optimisation avoids calling an expensive sets:subtract/2 when there is a single new confirm and instead does a sets:del_element/2 of the first set.
1 parent b623f8e commit 0dce848

File tree

2 files changed

+23
-6
lines changed

2 files changed

+23
-6
lines changed

deps/rabbit/src/rabbit_msg_store.erl

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -936,8 +936,11 @@ handle_cast({remove, CRef, MsgIds}, State) ->
936936
ignore -> {Removed, State2}
937937
end
938938
end, {[], State}, MsgIds),
939-
noreply(maybe_compact(client_confirm(CRef, sets:from_list(RemovedMsgIds, [{version, 2}]),
940-
ignored, State1)));
939+
case RemovedMsgIds of
940+
[] -> noreply(State1);
941+
_ -> noreply(maybe_compact(client_confirm(CRef, sets:from_list(RemovedMsgIds, [{version, 2}]),
942+
ignored, State1)))
943+
end;
941944

942945
handle_cast({combine_files, Source, Destination, Reclaimed},
943946
State = #msstate { sum_file_size = SumFileSize,
@@ -1372,7 +1375,7 @@ client_confirm(CRef, MsgIds, ActionTaken, State) ->
13721375
case maps:find(CRef, CTM) of
13731376
{ok, Gs} -> MsgOnDiskFun(sets:intersection(Gs, MsgIds),
13741377
ActionTaken),
1375-
MsgIds1 = sets:subtract(Gs, MsgIds),
1378+
MsgIds1 = sets_subtract(Gs, MsgIds),
13761379
case sets:is_empty(MsgIds1) of
13771380
true -> maps:remove(CRef, CTM);
13781381
false -> maps:put(CRef, MsgIds1, CTM)
@@ -1381,6 +1384,13 @@ client_confirm(CRef, MsgIds, ActionTaken, State) ->
13811384
end
13821385
end, CRef, State).
13831386

1387+
%% Function defined in both rabbit_msg_store and rabbit_variable_queue.
1388+
sets_subtract(Set1, Set2) ->
1389+
case sets:size(Set2) of
1390+
1 -> sets:del_element(hd(sets:to_list(Set2)), Set1);
1391+
_ -> sets:subtract(Set1, Set2)
1392+
end.
1393+
13841394
blind_confirm(CRef, MsgIds, ActionTaken, State) ->
13851395
update_pending_confirms(
13861396
fun (MsgOnDiskFun, CTM) -> MsgOnDiskFun(MsgIds, ActionTaken), CTM end,

deps/rabbit/src/rabbit_variable_queue.erl

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2355,11 +2355,18 @@ record_confirms(MsgIdSet, State = #vqstate { msgs_on_disk = MOD,
23552355
unconfirmed = UC,
23562356
confirmed = C }) ->
23572357
State #vqstate {
2358-
msgs_on_disk = sets:subtract(MOD, MsgIdSet),
2359-
msg_indices_on_disk = sets:subtract(MIOD, MsgIdSet),
2360-
unconfirmed = sets:subtract(UC, MsgIdSet),
2358+
msgs_on_disk = sets_subtract(MOD, MsgIdSet),
2359+
msg_indices_on_disk = sets_subtract(MIOD, MsgIdSet),
2360+
unconfirmed = sets_subtract(UC, MsgIdSet),
23612361
confirmed = sets:union(C, MsgIdSet) }.
23622362

2363+
%% Function defined in both rabbit_msg_store and rabbit_variable_queue.
2364+
sets_subtract(Set1, Set2) ->
2365+
case sets:size(Set2) of
2366+
1 -> sets:del_element(hd(sets:to_list(Set2)), Set1);
2367+
_ -> sets:subtract(Set1, Set2)
2368+
end.
2369+
23632370
msgs_written_to_disk(Callback, MsgIdSet, ignored) ->
23642371
Callback(?MODULE,
23652372
fun (?MODULE, State) -> record_confirms(MsgIdSet, State) end);

0 commit comments

Comments
 (0)