You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/03/11 22:39:06 UTC

[GitHub] [activemq-artemis] clebertsuconic opened a new pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

clebertsuconic opened a new pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598207330
 
 
   Do you have teams? hangouts? or bluejeans can do any

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#discussion_r391617834
 
 

 ##########
 File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java
 ##########
 @@ -530,11 +529,6 @@ void slowConsumerDetected(String sessionID,
    @Message(id = 222023, value = "problem cleaning page address {0}", format = Message.Format.MESSAGE_FORMAT)
    void problemCleaningPageAddress(@Cause Exception e, SimpleString address);
 
-   @LogMessage(level = Logger.Level.WARN)
-   @Message(id = 222024, value = "Could not complete operations on IO context {0}",
 
 Review comment:
   On another fix there was a loop removed here.. the thread is exausting the pool and causing the log.warn.
   
   
   notice I'm also fixing the context used on cleanup.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598200164
 
 
   Just watching logs like a hawk atm whilst testing this, we're not using large messages, and likewise there is no ids, but with master + this pr, i see in the logs the below, which is spurious (i dont think its this PR, but something to check - and maybe fix, not sure what introduced it)
   
   2020-03-12 13:54:30,271 INFO  [org.apache.activemq.artemis.core.server] AMQ224100: Timed out waiting for large messages deletion with IDs [], might not be deleted if broker crashes atm
   2020-03-12 13:54:35,272 INFO  [org.apache.activemq.artemis.core.server] AMQ224100: Timed out waiting for large messages deletion with IDs [], might not be deleted if broker crashes atm

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#discussion_r391617038
 
 

 ##########
 File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java
 ##########
 @@ -517,6 +528,23 @@ protected void cleanupComplete(ArrayList<PageSubscription> cursorList) throws Ex
 
       storeBookmark(cursorList, currentPage);
 
+      // notice this will be using the OperationContext started by cleanup
+      storageManager.afterCompleteOperations(new IOCallback() {
+         @Override
+         public void done() {
+
+            for (PageSubscription cursor : cursorList) {
+               cursor.enableAutoCleanup();
+            }
+         }
+
+         @Override
+         public void onError(int errorCode, String errorMessage) {
 
 Review comment:
   some place else would be already listening for it and the broker would shutdown on critical error.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#discussion_r391616848
 
 

 ##########
 File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java
 ##########
 @@ -530,11 +529,6 @@ void slowConsumerDetected(String sessionID,
    @Message(id = 222023, value = "problem cleaning page address {0}", format = Message.Format.MESSAGE_FORMAT)
    void problemCleaningPageAddress(@Cause Exception e, SimpleString address);
 
-   @LogMessage(level = Logger.Level.WARN)
-   @Message(id = 222024, value = "Could not complete operations on IO context {0}",
 
 Review comment:
   No... we are using asynchronously now.. before it was blocking.. no need to the handler.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain edited a comment on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain edited a comment on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598206055
 
 
   so the simulation test, was a way we re-created what occured in prod, basically we had a massive volatilty in the market meaning alot of messages burst into the session, it took a we while for consumers to scale up causing to page, but then consumers kept pace, but then we found we had some blocked flows.
   
   Note the recreator basically is a way we've found to re-create the problem, i expect in issues like this decreased perfomance but i dont expect it to stop and block.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#discussion_r391615802
 
 

 ##########
 File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java
 ##########
 @@ -517,6 +528,23 @@ protected void cleanupComplete(ArrayList<PageSubscription> cursorList) throws Ex
 
       storeBookmark(cursorList, currentPage);
 
+      // notice this will be using the OperationContext started by cleanup
+      storageManager.afterCompleteOperations(new IOCallback() {
+         @Override
+         public void done() {
+
+            for (PageSubscription cursor : cursorList) {
+               cursor.enableAutoCleanup();
+            }
+         }
+
+         @Override
+         public void onError(int errorCode, String errorMessage) {
 
 Review comment:
   what about error handling?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598201682
 
 
   happy to have a session for you to see first hand....

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#discussion_r391619974
 
 

 ##########
 File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java
 ##########
 @@ -530,11 +529,6 @@ void slowConsumerDetected(String sessionID,
    @Message(id = 222023, value = "problem cleaning page address {0}", format = Message.Format.MESSAGE_FORMAT)
    void problemCleaningPageAddress(@Cause Exception e, SimpleString address);
 
-   @LogMessage(level = Logger.Level.WARN)
-   @Message(id = 222024, value = "Could not complete operations on IO context {0}",
 
 Review comment:
   just deployed to real server in testing env, seeing if issue of flow stopping is resolved. will let you know

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598203546
 
 
   so the flowcontrol one is merged to master i saw, so i locally merged this fix with master and then built

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598201201
 
 
   fyi, this latest doesnt seem to fix the issue with the flow stopping, it seems from what i can tell is the producers get blocked
   
   ![image](https://user-images.githubusercontent.com/50627341/76529085-b2a93a00-6469-11ea-9efa-ab3a2d619a16.png)
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598211495
 
 
   sent a teams invite to your apache.org email

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#discussion_r391616057
 
 

 ##########
 File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java
 ##########
 @@ -612,26 +640,15 @@ protected void onDeletePage(Page deletedPage) throws Exception {
    }
 
    /**
+    * This method will store the last known position for paging right before we stopped paging.
     * @param cursorList
     * @param currentPage
     * @throws Exception
     */
    protected void storeBookmark(ArrayList<PageSubscription> cursorList, Page currentPage) throws Exception {
-      try {
-         // First step: Move every cursor to the next bookmarked page (that was just created)
-         for (PageSubscription cursor : cursorList) {
-            cursor.confirmPosition(new PagePositionImpl(currentPage.getPageId(), -1));
-         }
-
-         // we just need to make sure the storage is done..
-         // if the thread pool is full, we will just log it once instead of looping
-         if (!storageManager.waitOnOperations(5000)) {
-            ActiveMQServerLogger.LOGGER.problemCompletingOperations(storageManager.getContext());
-         }
-      } finally {
-         for (PageSubscription cursor : cursorList) {
-            cursor.enableAutoCleanup();
-         }
+      // Move every cursor to the next bookmarked page
 
 Review comment:
   how now are we making sure storage is done?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#discussion_r391642624
 
 

 ##########
 File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageCursorProviderImpl.java
 ##########
 @@ -612,26 +640,15 @@ protected void onDeletePage(Page deletedPage) throws Exception {
    }
 
    /**
+    * This method will store the last known position for paging right before we stopped paging.
     * @param cursorList
     * @param currentPage
     * @throws Exception
     */
    protected void storeBookmark(ArrayList<PageSubscription> cursorList, Page currentPage) throws Exception {
-      try {
-         // First step: Move every cursor to the next bookmarked page (that was just created)
-         for (PageSubscription cursor : cursorList) {
-            cursor.confirmPosition(new PagePositionImpl(currentPage.getPageId(), -1));
-         }
-
-         // we just need to make sure the storage is done..
-         // if the thread pool is full, we will just log it once instead of looping
-         if (!storageManager.waitOnOperations(5000)) {
-            ActiveMQServerLogger.LOGGER.problemCompletingOperations(storageManager.getContext());
-         }
-      } finally {
-         for (PageSubscription cursor : cursorList) {
-            cursor.enableAutoCleanup();
-         }
+      // Move every cursor to the next bookmarked page
 
 Review comment:
   I'm setting on a callback on the storage 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598203063
 
 
   @michaelandrepearce did you apply the other fix for AMQP Flow control? did you get master + this fix?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic closed pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
clebertsuconic closed pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on a change in pull request #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#discussion_r391616285
 
 

 ##########
 File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServerLogger.java
 ##########
 @@ -530,11 +529,6 @@ void slowConsumerDetected(String sessionID,
    @Message(id = 222023, value = "problem cleaning page address {0}", format = Message.Format.MESSAGE_FORMAT)
    void problemCleaningPageAddress(@Cause Exception e, SimpleString address);
 
-   @LogMessage(level = Logger.Level.WARN)
-   @Message(id = 222024, value = "Could not complete operations on IO context {0}",
 
 Review comment:
   this just removes the log line we are seeing, isnt that now just masking the problem?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598204837
 
 
   @michaelandrepearce 
   
   ```
   2020-03-12 13:54:30,271 INFO [org.apache.activemq.artemis.core.server] AMQ224100: Timed out waiting for large messages deletion with IDs [], might not be deleted if broker crashes atm
   ```
   
   This is another place doing the same thing I did before.. 
   
   One thing to notice is you are starting to page.. and then depaging really fast.. why is that happening?
   
   I can fix this blocking issue, but you should also look at that cause.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598206987
 
 
   How can we talk offline?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
michaelpearce-gain commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-598206055
 
 
   so the simulation test, was a way we re-created what occured in prod, basically we had a massive volatilty in the market meaning alot of messages burst into the session, it took a we while for consumers to scale up causing to page, but then we had some blocked flow, the recreator basically is a way we've found to re-create the problem

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3015: ARTEMIS-2654 Improving Page cleanup and Operation Context
URL: https://github.com/apache/activemq-artemis/pull/3015#issuecomment-597952284
 
 
   @michaelandrepearce I think this is probably more likely to be your issue.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services