Have circuit breaker succeed on unknown mem usage #33125
Conversation
With this commit we implement a workaround for https://bugs.openjdk.java.net/browse/JDK-8207200 which is a race condition in the JVM that results in `IllegalArgumentException` to be thrown in rare cases when we determine memory usage via `MemoryMXBean`. As we do not want to fail requests in those cases we always return zero memory usage. Relates elastic#31767
danielmitterdorfer
added
>enhancement
review
:Core/Infra/Circuit Breakers
|
Pinging @elastic/es-core-infra |
| return MEMORY_MX_BEAN.getHeapMemoryUsage().getUsed(); | ||
| try { | ||
| return MEMORY_MX_BEAN.getHeapMemoryUsage().getUsed(); | ||
| } catch (IllegalArgumentException ex) { |
There was a problem hiding this comment.
Neither of the methods here document that they can throw an IllegalArgumentException. So this catch looks quite odd and now I wonder if it's too broad. Catching it is fine, but can also check the exception message and assert that it is what we expect it to be? I don't want us to end up suppressing anything else here.
There was a problem hiding this comment.
The IllegalArgumentException is thrown by the constructor of MemoryUsage. It does contain a few more precondition checks but I can add an assert. However, for this we need to resort to pattern matching as the message contains absolute numbers, e.g:
committed = 542113792 should be < max = 536870912
There was a problem hiding this comment.
I understand where it's coming from. I am saying the Javadocs for MemoryMXBean#getHeapMemoryUsage do not relay this fact. That it's coming from a constructor is an implementation detail.
There was a problem hiding this comment.
Ah ok. Then I just misunderstood. Anyway, I can implement the assertion as suggested.
With this commit we implement a workaround for
https://bugs.openjdk.java.net/browse/JDK-8207200 which is a race
condition in the JVM that results in
IllegalArgumentExceptionto bethrown in rare cases when we determine memory usage via
MemoryMXBean.As we do not want to fail requests in those cases we always return zero
memory usage.
Relates #31767