cshannon commented on code in PR #1355:
URL: https://github.com/apache/activemq/pull/1355#discussion_r1852341486


##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/cursors/AbstractStoreCursor.java:
##########
@@ -255,12 +255,20 @@ public synchronized boolean 
tryAddMessageLast(MessageReference node, long wait)
             disableCache = true;
         }
 
-        if (disableCache && isCacheEnabled()) {
+        // AMQ-9625 - use this.cacheEnabled directly because the method 
isCacheEnabled() is overriden
+        // to try to re-enable the cache which we don't want at this point as 
we already skipped
+        // adding it to the cache
+        if (disableCache && this.cacheEnabled) {
             if (LOG.isTraceEnabled()) {

Review Comment:
   I don't see value in a metric because we already know if this is occurring 
frequently based on memory usage and adding a metric wouldn't really help with 
tuning because it doesn't have anything to do with the problem. 
   
   If this is happening, the problem that needs tuning is consumers are not 
fast enough and memory is full which is already easily determined by existing 
logging and tracking for when the cache is disabled and memory thresholds are 
hit. The admins should be paying attention to the memory being full as that is 
the root cause. If the memory doesn't fill up then the cache won't be disabled.
   
   This fix is just for the race condition on the pending writes and adding 
another metric here doesn't really help an admin with any real useful 
information they would not already have. We are essentially just fixing a bug 
that happens because memory is full but it doesn't really effect anything with 
tuning.



-- 
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-unsubscr...@activemq.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact


Reply via email to