Skip to content

Conversation

@ulischulte
Copy link
Contributor

Closes #2897


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());
Copy link

@cdprete cdprete Oct 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

@ulischulte ulischulte force-pushed the fix/2897-prevent-overflow-on-repeated-failures branch from 68d6cef to dfd7bfd Compare October 25, 2025 21:54
@ulischulte ulischulte requested a review from Copilot November 7, 2025 15:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the IntervalCheck service to handle backpressure and prevent overflow exceptions under high load or slow check operations. The changes introduce reactive backpressure handling using onBackpressureLatest() and switch from sequential to concurrent processing with flatMap.

Key changes:

  • Switched from concatMap to flatMap with controlled concurrency to allow parallel check execution
  • Added onBackpressureLatest() operator to prevent dropped checks and overflow exceptions under backpressure
  • Exposed retryConsumer setter and lastChecked getter for improved testability
  • Added comprehensive tests for overflow scenarios and backpressure handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File Description
spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/IntervalCheck.java Enhanced with backpressure handling via onBackpressureLatest(), switched to concurrent processing with flatMap, and exposed internal state for testing
spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/IntervalCheckTest.java Added three new tests to verify no overflow exceptions occur and checks aren't lost under backpressure scenarios
Comments suppressed due to low confidence (1)

spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/IntervalCheck.java:52

	@Getter

ulischulte and others added 9 commits November 7, 2025 17:01
…in/server/services/IntervalCheckTest.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…in/server/services/IntervalCheckTest.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…in/server/services/IntervalCheckTest.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…in/server/services/IntervalCheckTest.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…in/server/services/IntervalCheckTest.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ulischulte ulischulte marked this pull request as ready for review November 7, 2025 16:39
@ulischulte ulischulte requested a review from a team as a code owner November 7, 2025 16:39
@ulischulte ulischulte requested a review from SteKoe November 10, 2025 07:47
@SteKoe SteKoe merged commit 5734b2f into master Nov 18, 2025
2 checks passed
@SteKoe SteKoe deleted the fix/2897-prevent-overflow-on-repeated-failures branch November 18, 2025 17:59
ahoehma pushed a commit to ahoehma/spring-boot-admin that referenced this pull request Nov 28, 2025
…edation under high load (codecentric#4784)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backoff retry delay in status check is increasing to hours rendering services offline

4 participants