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