Added DefaultMemoryHealthChecker #4711
Conversation
|
|
core/src/main/java/com/linecorp/armeria/server/healthcheck/DefaultMemoryHealthChecker.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/healthcheck/DefaultMemoryHealthChecker.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/healthcheck/DefaultMemoryHealthChecker.java
Outdated
Show resolved
Hide resolved
|
|
||
| public class DefaultMemoryHealthChecker implements HealthChecker { | ||
|
|
||
| private final double targetHeapMemoryUtilizationRate; |
There was a problem hiding this comment.
Could we change Rate to Ratio in this PR? Rate means speed of change which is not what we meant here.
| this.targetHeapMemoryUtilizationRate = targetHeapMemoryUtilizationRate; | ||
| this.targetNonHeapMemoryUtilizationRate = targetNonHeapMemoryUtilizationRate; | ||
| this.targetTotalMemoryUtilizationRate = targetMemoryTotalUsage; |
There was a problem hiding this comment.
Yeah, we could add some checks here, e.g.
this.heapUsageRatioThreshold = validateRatio(heapUsageRatioThreshold, "heapUsageRatioThreshold");
this.nonHeapUsageRatioThreshold = validateRatio(nonHeapUsageRatioThreshold, "nonHeapUsageRatioThreshold");
this.totalUsageRatioThreshold = validateRatio(totalUsageRatioThreshold, "totalUsageRatioThreshold");
...
private static void validateRatio(double ratio, String paramName) {
checkArgument(ratio >= 0 && ratio <= 1, "%s (expected: >= 0 and <= 1)", paramName);
}Also, I suggested renaming the parameters and member fields as shown in the above example.
- Utilization -> Usage (because it's shorter)
- Memory -> '' (because it's already a part of the enclosing class name)
- Target -> Threshold (because target means something we aim for, but we don't really want that happen here)
| final double totalMemoryUsage = Runtime.getRuntime().totalMemory(); | ||
| return (heapMemoryUsage / maximumHeapMemory) <= targetHeapMemoryUtilizationRate && | ||
| (nonHeapMemoryUsage == null || (nonHeapMemoryUsage.getMemoryUsed() / nonHeapMemoryUsage.getTotalCapacity()) <= targetNonHeapMemoryUtilizationRate) && | ||
| (runtimeMaxMemory == Long.MAX_VALUE || totalMemoryUsage / runtimeMaxMemory <= targetTotalMemoryUtilizationRate); |
There was a problem hiding this comment.
Can we log a warning message if nonHeapMemoryUsage is null or runtimeMaxMemory is Long.MAX_VALUE, so that we tell our users that those properties are not available and thus it will never become unhealthy even if you specified the threshold?
We could keep a boolean flag so make sure that the log message is logged only once, so we don't pollute the log file.
| } | ||
|
|
||
| private List<MemoryPoolMXBean> getHeapMemories() { | ||
| return ManagementFactory.getMemoryPoolMXBeans().stream().filter(e -> MemoryType.HEAP.equals(e.getType())).collect(Collectors.toList()); |
There was a problem hiding this comment.
We could call .map(MemoryPoolMXBean::getUsage) before collecting so we don't have to do it twice in isHealthy().
| final BufferPoolMXBean nonHeapMemoryUsage = ManagementFactory.getPlatformMXBean(BufferPoolMXBean.class); | ||
| final double totalMemoryUsage = Runtime.getRuntime().totalMemory(); | ||
| return (heapMemoryUsage / maximumHeapMemory) <= targetHeapMemoryUtilizationRate && | ||
| (nonHeapMemoryUsage == null || (nonHeapMemoryUsage.getMemoryUsed() / nonHeapMemoryUsage.getTotalCapacity()) <= targetNonHeapMemoryUtilizationRate) && |
There was a problem hiding this comment.
Convert from long to double before dividing?
| @@ -0,0 +1,51 @@ | |||
| package com.linecorp.armeria.server.healthcheck; | |||
|
|
||
| import com.google.common.base.MoreObjects; | ||
|
|
||
| public class DefaultMemoryHealthChecker implements HealthChecker { |
There was a problem hiding this comment.
Could be package private final:
| public class DefaultMemoryHealthChecker implements HealthChecker { | |
| final class DefaultMemoryHealthChecker implements HealthChecker { |
|
gentle ping @lazmond3 😄 |
Motivation:
Resolves part of #1854
Some users may want to use Memory usage to determine if their instance is healthy.
Modifications:
DefaultMemoryHealthCheckerclass that checks memory-usage according to the method shown below.Runtime.getRuntime().totalMemory()ManagementFactory.getPlatformMXBean(BufferPoolMXBean.class).getMemoryUsed()ManagementFactory.getMemoryPoolMXBeans()(filtered by its memory type heap)Result:
HealthCheckerimplementations #1854