Added backpressure handling to prevent memory buildup and system degredation under high load #4784
Conversation
|
|
||
| private Retry createRetrySpec() { | ||
| return Retry.backoff(Long.MAX_VALUE, Duration.ofSeconds(1)).maxBackoff(maxBackoff).doBeforeRetry((s) -> { | ||
| log.warn("Unexpected error in {}-check", this.name, s.failure()); |
There was a problem hiding this comment.
@ulischulte isn't this line useless given that the errorConsumer will just do the same but on ERROR level?
In the previous code there was just a warning, but now you end up with a warning and and error providing exactly the same information.
Actually I find the WARN level better in this case, because it's just one of the retries.
There was a problem hiding this comment.
I wouldn't say useless. In the first step, I more or less adapted the error logging from the previous implementation. It also logs a warning in doBeforeRetry AND has a direct implementation of the errorConsumer in subscribe.
Please note, however, that you are reviewing a WIP/draft that has not yet been released for review.
I am in the process of creating failing tests for the known scenarios and, in the first step, I intentionally injected an errorConsumer for easier verifiability.
There was a problem hiding this comment.
I am in the process of creating failing tests for the known scenarios and, in the first step, I intentionally injected an errorConsumer for easier verifiability.
Ok, I see.
I wanted just to avoid to have double entries in my logs. :)
There was a problem hiding this comment.
And please keep in mind. I didn't change the error logging here so far, regardless of wether it's verbose or not. The old and new IntervalCheck both log a warning before retry and both invoke an errorConsumer when ever there's an error signal.
There was a problem hiding this comment.
Yep. My comment was only about the code block that gets executed before the retry.
Now it logs a warning, the previous line, and it invokes as well the error consumer, which logs an error.
Or, that's what I see in the diff here honestly :)
…, preventing lost checks under overload.
…, preventing lost checks under overload.
ulischulte
force-pushed
the
fix/2897-prevent-overflow-on-repeated-failures
branch
from
68d6cef to
dfd7bfd
Compare
…erflow-on-repeated-failures
# Conflicts: # spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/IntervalCheckTest.java
Closes #2897