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 2021/09/28 10:23:03 UTC

[GitHub] [activemq-artemis] AntonRoskvist opened a new pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

AntonRoskvist opened a new pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776


   …p broker from starting with currupted messages


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] asfgit closed pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776


   


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] AntonRoskvist commented on a change in pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on a change in pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#discussion_r718805550



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/PostOfficeJournalLoader.java
##########
@@ -221,23 +221,27 @@ public void handleAddMessage(Map<Long, Map<Long, AddMessageRecord>> queueMap) th
          long currentTime = System.currentTimeMillis();
 
          for (AddMessageRecord record : valueRecords) {
-            long scheduledDeliveryTime = record.getScheduledDeliveryTime();
+            try {
+               long scheduledDeliveryTime = record.getScheduledDeliveryTime();
 
-            if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
-               scheduledDeliveryTime = 0;
-               record.getMessage().setScheduledDeliveryTime(0L);
-            }
+               if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
+                  scheduledDeliveryTime = 0;
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
-            }
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
+               }
 
-            MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
+               MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
 
-            ref.setDeliveryCount(record.getDeliveryCount());
+               ref.setDeliveryCount(record.getDeliveryCount());
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(0L);
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
+            } catch (Throwable e) {

Review comment:
       Yes that makes sense, sounds good




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] AntonRoskvist commented on a change in pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on a change in pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#discussion_r718805550



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/PostOfficeJournalLoader.java
##########
@@ -221,23 +221,27 @@ public void handleAddMessage(Map<Long, Map<Long, AddMessageRecord>> queueMap) th
          long currentTime = System.currentTimeMillis();
 
          for (AddMessageRecord record : valueRecords) {
-            long scheduledDeliveryTime = record.getScheduledDeliveryTime();
+            try {
+               long scheduledDeliveryTime = record.getScheduledDeliveryTime();
 
-            if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
-               scheduledDeliveryTime = 0;
-               record.getMessage().setScheduledDeliveryTime(0L);
-            }
+               if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
+                  scheduledDeliveryTime = 0;
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
-            }
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
+               }
 
-            MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
+               MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
 
-            ref.setDeliveryCount(record.getDeliveryCount());
+               ref.setDeliveryCount(record.getDeliveryCount());
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(0L);
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
+            } catch (Throwable e) {

Review comment:
       Yes that makes sense, sounds good




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] AntonRoskvist commented on a change in pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on a change in pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#discussion_r717845556



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/PostOfficeJournalLoader.java
##########
@@ -221,23 +221,27 @@ public void handleAddMessage(Map<Long, Map<Long, AddMessageRecord>> queueMap) th
          long currentTime = System.currentTimeMillis();
 
          for (AddMessageRecord record : valueRecords) {
-            long scheduledDeliveryTime = record.getScheduledDeliveryTime();
+            try {
+               long scheduledDeliveryTime = record.getScheduledDeliveryTime();
 
-            if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
-               scheduledDeliveryTime = 0;
-               record.getMessage().setScheduledDeliveryTime(0L);
-            }
+               if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
+                  scheduledDeliveryTime = 0;
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
-            }
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
+               }
 
-            MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
+               MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
 
-            ref.setDeliveryCount(record.getDeliveryCount());
+               ref.setDeliveryCount(record.getDeliveryCount());
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(0L);
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
+            } catch (Throwable e) {

Review comment:
       This is preceded by a massive dump about everything related to the message, so it seemed a bit excessive. An example of that can be seen in the attached log in the ticket, but it includes messageID, why it failed and the entire message bytebuffer (pretty printed)

##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/PostOfficeJournalLoader.java
##########
@@ -221,23 +221,27 @@ public void handleAddMessage(Map<Long, Map<Long, AddMessageRecord>> queueMap) th
          long currentTime = System.currentTimeMillis();
 
          for (AddMessageRecord record : valueRecords) {
-            long scheduledDeliveryTime = record.getScheduledDeliveryTime();
+            try {
+               long scheduledDeliveryTime = record.getScheduledDeliveryTime();
 
-            if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
-               scheduledDeliveryTime = 0;
-               record.getMessage().setScheduledDeliveryTime(0L);
-            }
+               if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
+                  scheduledDeliveryTime = 0;
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
-            }
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
+               }
 
-            MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
+               MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
 
-            ref.setDeliveryCount(record.getDeliveryCount());
+               ref.setDeliveryCount(record.getDeliveryCount());
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(0L);
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
+            } catch (Throwable e) {

Review comment:
       (Thrown by org.apache.activemq.artemis.core.message.impl.CoreMessage #onCheckPropertiesError)




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#discussion_r718759120



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/PostOfficeJournalLoader.java
##########
@@ -221,23 +221,27 @@ public void handleAddMessage(Map<Long, Map<Long, AddMessageRecord>> queueMap) th
          long currentTime = System.currentTimeMillis();
 
          for (AddMessageRecord record : valueRecords) {
-            long scheduledDeliveryTime = record.getScheduledDeliveryTime();
+            try {
+               long scheduledDeliveryTime = record.getScheduledDeliveryTime();
 
-            if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
-               scheduledDeliveryTime = 0;
-               record.getMessage().setScheduledDeliveryTime(0L);
-            }
+               if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
+                  scheduledDeliveryTime = 0;
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
-            }
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
+               }
 
-            MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
+               MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
 
-            ref.setDeliveryCount(record.getDeliveryCount());
+               ref.setDeliveryCount(record.getDeliveryCount());
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(0L);
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
+            } catch (Throwable e) {

Review comment:
       Details about the failure are logged in this specific case, but there's no guarantee that will be the case for _every_ type of failure. These details still need to be logged. I'll amend the commit before I merge it with the requisite logging.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] AntonRoskvist commented on a change in pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on a change in pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#discussion_r717857720



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/PostOfficeJournalLoader.java
##########
@@ -221,23 +221,27 @@ public void handleAddMessage(Map<Long, Map<Long, AddMessageRecord>> queueMap) th
          long currentTime = System.currentTimeMillis();
 
          for (AddMessageRecord record : valueRecords) {
-            long scheduledDeliveryTime = record.getScheduledDeliveryTime();
+            try {
+               long scheduledDeliveryTime = record.getScheduledDeliveryTime();
 
-            if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
-               scheduledDeliveryTime = 0;
-               record.getMessage().setScheduledDeliveryTime(0L);
-            }
+               if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
+                  scheduledDeliveryTime = 0;
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
-            }
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
+               }
 
-            MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
+               MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
 
-            ref.setDeliveryCount(record.getDeliveryCount());
+               ref.setDeliveryCount(record.getDeliveryCount());
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(0L);
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
+            } catch (Throwable e) {

Review comment:
       (Thrown by org.apache.activemq.artemis.core.message.impl.CoreMessage #onCheckPropertiesError)




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#issuecomment-929515755


   At least in my case I end up in #handleNoMessageReferences (from AbstractJournalStorageManager) and the message is removed by:
   storageManager.deleteMessage(msg.getMessageID());
   
   And the following log:
   AMQ221019: Deleting unreferenced message id=$MESSAGEID from the journal
   


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] AntonRoskvist commented on a change in pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on a change in pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#discussion_r717845556



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/PostOfficeJournalLoader.java
##########
@@ -221,23 +221,27 @@ public void handleAddMessage(Map<Long, Map<Long, AddMessageRecord>> queueMap) th
          long currentTime = System.currentTimeMillis();
 
          for (AddMessageRecord record : valueRecords) {
-            long scheduledDeliveryTime = record.getScheduledDeliveryTime();
+            try {
+               long scheduledDeliveryTime = record.getScheduledDeliveryTime();
 
-            if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
-               scheduledDeliveryTime = 0;
-               record.getMessage().setScheduledDeliveryTime(0L);
-            }
+               if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
+                  scheduledDeliveryTime = 0;
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
-            }
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
+               }
 
-            MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
+               MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
 
-            ref.setDeliveryCount(record.getDeliveryCount());
+               ref.setDeliveryCount(record.getDeliveryCount());
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(0L);
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
+            } catch (Throwable e) {

Review comment:
       This is preceded by a massive dump about everything related to the message, so it seemed a bit excessive. An example of that can be seen in the attached log in the ticket, but it includes messageID, why it failed and the entire message bytebuffer (pretty printed)




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#issuecomment-929059811






-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#discussion_r717689972



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/PostOfficeJournalLoader.java
##########
@@ -221,23 +221,27 @@ public void handleAddMessage(Map<Long, Map<Long, AddMessageRecord>> queueMap) th
          long currentTime = System.currentTimeMillis();
 
          for (AddMessageRecord record : valueRecords) {
-            long scheduledDeliveryTime = record.getScheduledDeliveryTime();
+            try {
+               long scheduledDeliveryTime = record.getScheduledDeliveryTime();
 
-            if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
-               scheduledDeliveryTime = 0;
-               record.getMessage().setScheduledDeliveryTime(0L);
-            }
+               if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
+                  scheduledDeliveryTime = 0;
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
-            }
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
+               }
 
-            MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
+               MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
 
-            ref.setDeliveryCount(record.getDeliveryCount());
+               ref.setDeliveryCount(record.getDeliveryCount());
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(0L);
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
+            } catch (Throwable e) {

Review comment:
       logger.warn(e.getMessage, e); ?
   
   
   some context about the message that is causing it?




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] jbertram commented on a change in pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
jbertram commented on a change in pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#discussion_r718759120



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/PostOfficeJournalLoader.java
##########
@@ -221,23 +221,27 @@ public void handleAddMessage(Map<Long, Map<Long, AddMessageRecord>> queueMap) th
          long currentTime = System.currentTimeMillis();
 
          for (AddMessageRecord record : valueRecords) {
-            long scheduledDeliveryTime = record.getScheduledDeliveryTime();
+            try {
+               long scheduledDeliveryTime = record.getScheduledDeliveryTime();
 
-            if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
-               scheduledDeliveryTime = 0;
-               record.getMessage().setScheduledDeliveryTime(0L);
-            }
+               if (scheduledDeliveryTime != 0 && scheduledDeliveryTime <= currentTime) {
+                  scheduledDeliveryTime = 0;
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
-            }
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(scheduledDeliveryTime);
+               }
 
-            MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
+               MessageReference ref = postOffice.reload(record.getMessage(), queue, null);
 
-            ref.setDeliveryCount(record.getDeliveryCount());
+               ref.setDeliveryCount(record.getDeliveryCount());
 
-            if (scheduledDeliveryTime != 0) {
-               record.getMessage().setScheduledDeliveryTime(0L);
+               if (scheduledDeliveryTime != 0) {
+                  record.getMessage().setScheduledDeliveryTime(0L);
+               }
+            } catch (Throwable e) {

Review comment:
       Details about the failure are logged in this specific case, but there's no guarantee that will be the case for _every_ type of failure. These details still need to be logged. I'll amend the commit before I merge it with the requisite logging.




-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] asfgit closed pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776


   


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3776: ARTEMIS-3501 Added exception handling on #handleAddMessage to not sto…

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3776:
URL: https://github.com/apache/activemq-artemis/pull/3776#issuecomment-929332171


   I don't think it will discard.. the message would still be in the journal until the next restart... right?


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org