From cc1bdfb8e4adeae4b55fa96e273ac2f44a32828f Mon Sep 17 00:00:00 2001 From: Erik Petzold Date: Fri, 17 Nov 2023 12:12:27 +0100 Subject: [PATCH] Bugfix/2897 limit exponential backoff (#2903) * #2897: WIP Fix exponential backoff * reduce number of places where defaults can be defined * use configured backoff in retry * #2897: javaformat * add Test * add docs * reduce number of places with defaults --------- Co-authored-by: ulrichschulte --- .../src/site/asciidoc/server.adoc | 8 ++++++++ .../config/AdminServerAutoConfiguration.java | 15 +++++++-------- .../server/config/AdminServerProperties.java | 14 ++++++++++++++ .../admin/server/services/InfoUpdateTrigger.java | 5 +++-- .../admin/server/services/IntervalCheck.java | 11 ++++++----- .../server/services/StatusUpdateTrigger.java | 6 ++++-- .../server/services/InfoUpdateTriggerTest.java | 3 ++- .../admin/server/services/IntervalCheckTest.java | 16 +++++++++++++++- .../server/services/StatusUpdateTriggerTest.java | 3 ++- 9 files changed, 61 insertions(+), 20 deletions(-) diff --git a/spring-boot-admin-docs/src/site/asciidoc/server.adoc b/spring-boot-admin-docs/src/site/asciidoc/server.adoc index 42ef3b90918..7bbd278d6b7 100644 --- a/spring-boot-admin-docs/src/site/asciidoc/server.adoc +++ b/spring-boot-admin-docs/src/site/asciidoc/server.adoc @@ -22,6 +22,10 @@ In addition when the reverse proxy terminates the https connection, it may be ne | Time interval to check the status of instances. | 10,000ms +| spring.boot.admin.monitor.status-max-backoff +| The maximal backoff for status check retries (retry after error has exponential backoff, minimum backoff is 1 second). +| 60,000ms + | spring.boot.admin.monitor.status-lifetime | Lifetime of status. The status won't be updated as long the last status isn't expired. | 10,000ms @@ -30,6 +34,10 @@ In addition when the reverse proxy terminates the https connection, it may be ne | Time interval to check the info of instances. | 1m +| spring.boot.admin.monitor.info-max-backoff +| The maximal backoff for info check retries (retry after error has exponential backoff, minimum backoff is 1 second). +| 10m + | spring.boot.admin.monitor.info-lifetime | Lifetime of info. The info won't be updated as long the last info isn't expired. | 1m diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/config/AdminServerAutoConfiguration.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/config/AdminServerAutoConfiguration.java index 849832ab315..406d24e3b37 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/config/AdminServerAutoConfiguration.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/config/AdminServerAutoConfiguration.java @@ -95,10 +95,10 @@ public StatusUpdater statusUpdater(InstanceRepository instanceRepository, @Bean(initMethod = "start", destroyMethod = "stop") @ConditionalOnMissingBean public StatusUpdateTrigger statusUpdateTrigger(StatusUpdater statusUpdater, Publisher events) { - StatusUpdateTrigger trigger = new StatusUpdateTrigger(statusUpdater, events); - trigger.setInterval(this.adminServerProperties.getMonitor().getStatusInterval()); - trigger.setLifetime(this.adminServerProperties.getMonitor().getStatusLifetime()); - return trigger; + return new StatusUpdateTrigger(statusUpdater, events, + this.adminServerProperties.getMonitor().getStatusInterval(), + this.adminServerProperties.getMonitor().getStatusLifetime(), + this.adminServerProperties.getMonitor().getStatusMaxBackoff()); } @Bean @@ -129,10 +129,9 @@ public InfoUpdater infoUpdater(InstanceRepository instanceRepository, @Bean(initMethod = "start", destroyMethod = "stop") @ConditionalOnMissingBean public InfoUpdateTrigger infoUpdateTrigger(InfoUpdater infoUpdater, Publisher events) { - InfoUpdateTrigger trigger = new InfoUpdateTrigger(infoUpdater, events); - trigger.setInterval(this.adminServerProperties.getMonitor().getInfoInterval()); - trigger.setLifetime(this.adminServerProperties.getMonitor().getInfoLifetime()); - return trigger; + return new InfoUpdateTrigger(infoUpdater, events, this.adminServerProperties.getMonitor().getInfoInterval(), + this.adminServerProperties.getMonitor().getInfoLifetime(), + this.adminServerProperties.getMonitor().getInfoMaxBackoff()); } @Bean diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/config/AdminServerProperties.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/config/AdminServerProperties.java index 7e85fdb7f57..7fcf13442b9 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/config/AdminServerProperties.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/config/AdminServerProperties.java @@ -103,12 +103,26 @@ public static class MonitorProperties { @DurationUnit(ChronoUnit.MILLIS) private Duration statusLifetime = Duration.ofMillis(10_000L); + /** + * The maximal backoff for status check retries (retry after error has exponential + * backoff, minimum backoff is 1 second). + */ + @DurationUnit(ChronoUnit.MILLIS) + private Duration statusMaxBackoff = Duration.ofMillis(60_000L); + /** * Time interval to check the info of instances, */ @DurationUnit(ChronoUnit.MILLIS) private Duration infoInterval = Duration.ofMinutes(1L); + /** + * The maximal backoff for info check retries (retry after error has exponential + * backoff, minimum backoff is 1 second). + */ + @DurationUnit(ChronoUnit.MILLIS) + private Duration infoMaxBackoff = Duration.ofMinutes(10); + /** * Lifetime of info. The info won't be updated as long the last info isn't * expired. diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/InfoUpdateTrigger.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/InfoUpdateTrigger.java index 0bf88b9a0be..fa7ce27b166 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/InfoUpdateTrigger.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/InfoUpdateTrigger.java @@ -38,10 +38,11 @@ public class InfoUpdateTrigger extends AbstractEventHandler { private final IntervalCheck intervalCheck; - public InfoUpdateTrigger(InfoUpdater infoUpdater, Publisher publisher) { + public InfoUpdateTrigger(InfoUpdater infoUpdater, Publisher publisher, Duration updateInterval, + Duration infoLifetime, Duration maxBackoff) { super(publisher, InstanceEvent.class); this.infoUpdater = infoUpdater; - this.intervalCheck = new IntervalCheck("info", this::updateInfo, Duration.ofMinutes(5), Duration.ofMinutes(1)); + this.intervalCheck = new IntervalCheck("info", this::updateInfo, updateInterval, infoLifetime, maxBackoff); } @Override diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/IntervalCheck.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/IntervalCheck.java index 22bb803b52f..9f982e13fcf 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/IntervalCheck.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/IntervalCheck.java @@ -52,6 +52,9 @@ public class IntervalCheck { private final Function> checkFn; + @Setter + private Duration maxBackoff; + @Getter @Setter private Duration interval; @@ -65,16 +68,13 @@ public class IntervalCheck { @Nullable private Scheduler scheduler; - public IntervalCheck(String name, Function> checkFn) { - this(name, checkFn, Duration.ofSeconds(10), Duration.ofSeconds(10)); - } - public IntervalCheck(String name, Function> checkFn, Duration interval, - Duration minRetention) { + Duration minRetention, Duration maxBackoff) { this.name = name; this.checkFn = checkFn; this.interval = interval; this.minRetention = minRetention; + this.maxBackoff = maxBackoff; } public void start() { @@ -85,6 +85,7 @@ public void start() { .subscribeOn(this.scheduler) .concatMap((i) -> this.checkAllInstances()) .retryWhen(Retry.backoff(Long.MAX_VALUE, Duration.ofSeconds(1)) + .maxBackoff(maxBackoff) .doBeforeRetry((s) -> log.warn("Unexpected error in {}-check", this.name, s.failure()))) .subscribe(null, (error) -> log.error("Unexpected error in {}-check", name, error)); } diff --git a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/StatusUpdateTrigger.java b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/StatusUpdateTrigger.java index 9d66bfeb0e2..80470f96957 100644 --- a/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/StatusUpdateTrigger.java +++ b/spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/StatusUpdateTrigger.java @@ -37,10 +37,12 @@ public class StatusUpdateTrigger extends AbstractEventHandler { private final IntervalCheck intervalCheck; - public StatusUpdateTrigger(StatusUpdater statusUpdater, Publisher publisher) { + public StatusUpdateTrigger(StatusUpdater statusUpdater, Publisher publisher, Duration updateInterval, + Duration statusLifetime, Duration maxBackoff) { super(publisher, InstanceEvent.class); this.statusUpdater = statusUpdater; - this.intervalCheck = new IntervalCheck("status", this::updateStatus); + this.intervalCheck = new IntervalCheck("status", this::updateStatus, updateInterval, statusLifetime, + maxBackoff); } @Override diff --git a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/InfoUpdateTriggerTest.java b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/InfoUpdateTriggerTest.java index 35aef65d99c..597ae00c32c 100644 --- a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/InfoUpdateTriggerTest.java +++ b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/InfoUpdateTriggerTest.java @@ -60,7 +60,8 @@ public class InfoUpdateTriggerTest { public void setUp() throws Exception { when(this.updater.updateInfo(any(InstanceId.class))).thenReturn(Mono.empty()); - this.trigger = new InfoUpdateTrigger(this.updater, this.events.flux()); + this.trigger = new InfoUpdateTrigger(this.updater, this.events.flux(), Duration.ofMinutes(5), Duration.ofMinutes(1), + Duration.ofMinutes(10)); this.trigger.start(); await().until(this.events::wasSubscribed); } diff --git a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/IntervalCheckTest.java b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/IntervalCheckTest.java index 4830c5aa9e0..5832ee47c1c 100644 --- a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/IntervalCheckTest.java +++ b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/IntervalCheckTest.java @@ -41,7 +41,7 @@ public class IntervalCheckTest { private final Function> checkFn = mock(Function.class, (i) -> Mono.empty()); private final IntervalCheck intervalCheck = new IntervalCheck("test", this.checkFn, Duration.ofMillis(10), - Duration.ofMillis(10)); + Duration.ofMillis(10), Duration.ofSeconds(1)); @Test public void should_check_after_being_started() throws InterruptedException { @@ -81,6 +81,20 @@ public void should_recheck_after_retention_period() throws InterruptedException verify(this.checkFn, atLeast(2)).apply(INSTANCE_ID); } + @Test + public void should_not_wait_longer_than_maxBackoff() throws InterruptedException { + this.intervalCheck.setInterval(Duration.ofMillis(10)); + this.intervalCheck.setMinRetention(Duration.ofMillis(10)); + this.intervalCheck.setMaxBackoff(Duration.ofSeconds(2)); + this.intervalCheck.markAsChecked(INSTANCE_ID); + + when(this.checkFn.apply(any())).thenReturn(Mono.error(new RuntimeException("Test"))); + + this.intervalCheck.start(); + Thread.sleep(1000 * 10); + verify(this.checkFn, atLeast(7)).apply(INSTANCE_ID); + } + @Test public void should_check_after_error() throws InterruptedException { this.intervalCheck.markAsChecked(INSTANCE_ID); diff --git a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/StatusUpdateTriggerTest.java b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/StatusUpdateTriggerTest.java index 88c44a5d152..de143d06776 100644 --- a/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/StatusUpdateTriggerTest.java +++ b/spring-boot-admin-server/src/test/java/de/codecentric/boot/admin/server/services/StatusUpdateTriggerTest.java @@ -58,7 +58,8 @@ public void setUp() throws Exception { when(this.updater.updateStatus(any(InstanceId.class))).thenReturn(Mono.empty()); when(this.updater.timeout(any())).thenReturn(this.updater); - this.trigger = new StatusUpdateTrigger(this.updater, this.events.flux()); + this.trigger = new StatusUpdateTrigger(this.updater, this.events.flux(), Duration.ofSeconds(10), + Duration.ofSeconds(10), Duration.ofSeconds(60)); this.trigger.start(); await().until(this.events::wasSubscribed); }