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 <[email protected]>

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

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<InstanceEvent> 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<InstanceEvent> 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

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.

spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/InfoUpdateTrigger.java

@@ -38,10 +38,11 @@ public class InfoUpdateTrigger extends AbstractEventHandler<InstanceEvent> {
 
 	private final IntervalCheck intervalCheck;
 
-	public InfoUpdateTrigger(InfoUpdater infoUpdater, Publisher<InstanceEvent> publisher) {
+	public InfoUpdateTrigger(InfoUpdater infoUpdater, Publisher<InstanceEvent> 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

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<InstanceId, Mono<Void>> 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<InstanceId, Mono<Void>> checkFn) {
-		this(name, checkFn, Duration.ofSeconds(10), Duration.ofSeconds(10));
-	}
-
 	public IntervalCheck(String name, Function<InstanceId, Mono<Void>> 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));
 	}

spring-boot-admin-server/src/main/java/de/codecentric/boot/admin/server/services/StatusUpdateTrigger.java

@@ -37,10 +37,12 @@ public class StatusUpdateTrigger extends AbstractEventHandler<InstanceEvent> {
 
 	private final IntervalCheck intervalCheck;
 
-	public StatusUpdateTrigger(StatusUpdater statusUpdater, Publisher<InstanceEvent> publisher) {
+	public StatusUpdateTrigger(StatusUpdater statusUpdater, Publisher<InstanceEvent> 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

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);
 	}

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<InstanceId, Mono<Void>> 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);

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);
 	}