clebertsuconic commented on code in PR #4029:
URL: https://github.com/apache/activemq-artemis/pull/4029#discussion_r850575993


##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java:
##########
@@ -1846,43 +1844,45 @@ private final class ExpiryReaper extends 
ActiveMQScheduledComponent {
          super(scheduledExecutorService, executor, checkPeriod, timeUnit, 
onDemand);
       }
 
-      volatile CountDownLatch inUseLatch;
+      private Iterator<Queue> iterator;
 
+      // this is for logger.debug only
+      private Queue currentQueue;
 
       @Override
-      public void stop() {
-         super.stop();
-         // this will do a best effort to stop the current latch.
-         // no big deal if it failed. this is just to optimize this component 
stop.
-         CountDownLatch latch = inUseLatch;
-         if (latch != null) {
-            latch.countDown();
+      public void run() {
+         if (iterator != null) {
+            logger.debugf("A previous reaping call has not finished yet, and 
it is currently working on %s", currentQueue);
+            return;
          }
+
+         iterator = iterableOf(getLocalQueues()).iterator();
+
+         moveNext();
       }
 
+      private void done() {
+         executor.execute(this::moveNext);
+      }
 
-      @Override
-      public void run() {
-         // The reaper thread should be finished case the PostOffice is gone
-         // This is to avoid leaks on PostOffice between stops and starts
-         for (Queue queue : iterableOf(getLocalQueues())) {
-            if (!isStarted()) {
-               break;
-            }
-            try {
-               CountDownLatch latch = new CountDownLatch(1);
-               this.inUseLatch = latch;
-               queue.expireReferences(latch::countDown);
-               // the idea is in fact to block the Reaper while the Queue is 
executing reaping.
-               // This would avoid another eventual expiry to be called if the 
period for reaping is too small
-               // This should also avoid bursts in CPU consumption because of 
the expiry reaping
-               if (!latch.await(10, TimeUnit.SECONDS)) {
-                  ActiveMQServerLogger.LOGGER.errorExpiringMessages(new 
TimeoutException(queue.getName().toString()));
-               }
-            } catch (Exception e) {
-               ActiveMQServerLogger.LOGGER.errorExpiringMessages(e);
-            }
+      private void moveNext() {
+         Queue queue;
+         if (!iterator.hasNext() || !this.isStarted()) {
+            queue = null;
+         } else {
+            queue = iterator.next();
          }
+
+         if (queue == null) {

Review Comment:
   I added the currentQueue just for debugging. but I guess it could have a 
double usage now...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to