add some comments and TODOs

1 files changed, 22 insertions, 15 deletions

diff --git a/src/rabbit_variable_queue.erl b/src/rabbit_variable_queue.erl
index 712ebf84d3..c1a67ddbaa 100644
--- a/src/rabbit_variable_queue.erl
+++ b/src/rabbit_variable_queue.erl
@@ -110,21 +110,18 @@
 %% should be very few betas remaining, thus the transition is fast (no
 %% work needs to be done for the gamma -> delta transition).
 %%
-%% The conversion of betas to gammas is done on publish, in batches of
-%% exactly ?RAM_INDEX_BATCH_SIZE. This value should not be too small,
-%% otherwise the frequent operations on the queues of q2 and q3 will
-%% not be effectively amortised, nor should it be too big, otherwise a
-%% publish will take too long as it attempts to do too much work and
-%% thus stalls the queue. Therefore, it must be just right. This
-%% approach is preferable to doing work on a new queue-duration
-%% because converting all the indicated betas to gammas at that point
-%% can be far too expensive, thus requiring batching and segmented
-%% work anyway, and furthermore, if we're not getting any publishes
-%% anyway then the queue is either being drained or has no
-%% consumers. In the latter case, an expensive beta to delta
-%% transition doesn't matter, and in the former case the queue's
-%% shrinking length makes it unlikely (though not impossible) that the
-%% duration will become 0.
+%% The conversion of betas to gammas is done on all actions that can
+%% increase the message count, such as publish and requeue, and when
+%% the queue is asked to reduce its memory usage. The conversion is
+%% done in batches of exactly ?RAM_INDEX_BATCH_SIZE. This value should
+%% not be too small, otherwise the frequent operations on the queues
+%% of q2 and q3 will not be effectively amortised (switching the
+%% direction of queue access defeats amortisation), nor should it be
+%% too big, otherwise converting a batch stalls the queue for too
+%% long. Therefore, it must be just right. This approach is preferable
+%% to doing work on a new queue-duration because converting all the
+%% indicated betas to gammas at that point can be far too expensive,
+%% thus requiring batching and segmented work anyway.
 %%
 %% In the queue we only keep track of messages that are pending
 %% delivery. This is fine for queue purging, but can be expensive for
@@ -393,6 +390,9 @@ terminate(State) ->
 %% the only difference between purge and delete is that delete also
 %% needs to delete everything that's been delivered and not ack'd.
 delete_and_terminate(State) ->
+    %% TODO: there is no need to interact with qi at all - which we do
+    %% as part of 'purge' and 'remove_pending_ack', other than
+    %% deleting it.
     {_PurgeCount, State1} = purge(State),
     State2 = #vqstate { index_state         = IndexState,
                         msg_store_clients   = {{MSCStateP, PRef},
@@ -411,6 +411,9 @@ delete_and_terminate(State) ->
                         msg_store_clients = undefined }).
 
 purge(State = #vqstate { q4 = Q4, index_state = IndexState, len = Len }) ->
+    %% TODO: when there are no pending acks, which is a common case,
+    %% we could simply wipe the qi instead of issuing delivers and
+    %% acks for all the messages.
     IndexState1 = remove_queue_entries(fun rabbit_misc:queue_fold/3, Q4,
                                        IndexState),
     State1 = #vqstate { q1 = Q1, index_state = IndexState2 } =
@@ -1133,6 +1136,10 @@ limit_ram_index(State = #vqstate { ram_index_count = RamIndexCount }) ->
                          {Q2a, {Reduction1, IndexState1}} =
                              limit_ram_index(fun bpqueue:map_fold_filter_l/4,
                                              Q2, {Reduction, IndexState}),
+                         %% TODO: we shouldn't be writing index
+                         %% entries for messages that can never end up
+                         %% in delta due them residing in the only
+                         %% segment held by q3.
                          {Q3a, {Reduction2, IndexState2}} =
                              limit_ram_index(fun bpqueue:map_fold_filter_r/4,
                                              Q3, {Reduction1, IndexState1}),