EdColeman commented on code in PR #3161:
URL: https://github.com/apache/accumulo/pull/3161#discussion_r1104914006
##########
server/base/src/main/java/org/apache/accumulo/server/mem/LowMemoryDetector.java:
##########
@@ -81,9 +139,19 @@ public void logGCInfo(AccumuloConfiguration conf) {
gcTimeIncreasedCount = 0;
} else {
gcTimeIncreasedCount++;
- if (gcTimeIncreasedCount > 3 && mem < rt.maxMemory() * 0.05) {
+ if (gcTimeIncreasedCount > 3 && mem < rt.maxMemory() *
freeMemoryPercentage) {
+ runningLowOnMemory = true;
log.warn("Running low on memory");
gcTimeIncreasedCount = 0;
+ } else {
+ // If we were running low on memory, but are not any longer, than
log at warn
+ // so that it shows up in the logs
+ if (runningLowOnMemory) {
+ log.warn("Not running low on memory");
+ } else {
+ log.debug("Not running low on memory");
+ }
Review Comment:
trace would be fine - and then addicting more info (current / threshold)
could be meaningful at that level.
As for the warning. Without context around the message (and that context
could be quite spread out in a busy log. It just seems odd to have a basically
message saying warning: nothing wrong. Taking it to an extreme for emphasis,
should I monitor things and know there's trouble if that message is not logged?
With the low memory conditions reported as warning, that seems reasonable.
Reporting the recovery at the same level could help with filtering, so that's
also reasonable. I'm just suggesting the message could be clearer - especially
when read as a stand-alone message.
--
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]
