Github user wy96f commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2369#discussion_r224976264
  
    --- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java
 ---
    @@ -599,6 +600,29 @@ private long checkMinPage(Collection<PageSubscription> 
cursorList) {
     
        }
     
    +   private void deliverIfNecessary(Collection<PageSubscription> 
cursorList) {
    +      long minPage = Long.MAX_VALUE;
    +      PageSubscription slowSubscription = null;
    +      int nonEmptyCursorNum = 0;
    +
    +      for (PageSubscription cursor : cursorList) {
    +         long firstPage = cursor.getFirstPage();
    +
    +         // the cursor will return -1 if the cursor is empty
    +         if (firstPage >= 0) {
    +            nonEmptyCursorNum++;
    +            if (firstPage < minPage) {
    +               minPage = firstPage;
    +               slowSubscription = cursor.getQueue().getMessageCount() == 0 
? cursor : null;
    +            }
    +         }
    +      }
    +
    +      if (nonEmptyCursorNum > 1 && slowSubscription != null) {
    +         slowSubscription.getQueue().deliverAsync();
    --- End diff --
    
    > Just to understand the fix: given that QueueImpl::deliverAsync will 
trigger QueueImpl::checkDepage that will schedule an async task 
QueueImpl::DepageRunner how do you know that it will be finished time in order 
to have the PageSubscription completed when it will check later on the same 
cleanup call?
    > From what I have understood it will be eventually completed on the 
cleanup that will follow a successfull DepageRunner::run...
    
    Yes, that's right. A successful DepageRunner::run will trigger 
PageSubscription::processACK, then PageSubscription::cleanupEntries, and lastly 
PageCursorProvider::cleanup.


---

Reply via email to