Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/2897 fix exponential backoff #2903

Merged
merged 7 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions spring-boot-admin-docs/src/site/asciidoc/server.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ public class IntervalCheck {

private final Function<InstanceId, Mono<Void>> checkFn;

@Setter
private Duration maxBackoff;

@Getter
@Setter
private Duration interval;
Expand All @@ -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() {
Expand All @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down