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.

Sign up for GitHub

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

Jump to bottom

Keep an endpoint's healthiness if long-poll health check returns NOT_… #2004

Merged
merged 2 commits into from Aug 21, 2019
Merged

Keep an endpoint's healthiness if long-poll health check returns NOT_… #2004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5de39b4
Keep an endpoint's healthiness if long-poll health check returns NOT_…
imasahiro Aug 20, 2019
f50338d
update
imasahiro Aug 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions core/src/main/java/com/linecorp/armeria/client/endpoint/healthcheck/HttpHealthChecker.java
View file
Open in desktop
Expand Up @@ -36,6 +36,7 @@
import com.linecorp.armeria.common.HttpMethod;
import com.linecorp.armeria.common.HttpRequest;
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.common.RequestHeaders;
import com.linecorp.armeria.common.RequestHeadersBuilder;
import com.linecorp.armeria.common.util.AsyncCloseable;
Expand Down Expand Up @@ -108,8 +109,13 @@ private synchronized void check() {
maxLongPollingSeconds = getMaxLongPollingSeconds(res);
break;
default:
// Do not use long polling on an unexpected status for safety.
maxLongPollingSeconds = 0;
if (res.status() == HttpStatus.NOT_MODIFIED) {
ikhoon marked this conversation as resolved.
Show resolved Hide resolved
maxLongPollingSeconds = getMaxLongPollingSeconds(res);
isHealthy = wasHealthy;
} else {
// Do not use long polling on an unexpected status for safety.
maxLongPollingSeconds = 0;
}
}
} else {
maxLongPollingSeconds = 0;
Expand Down
43 changes: 40 additions & 3 deletions ...rp/armeria/client/endpoint/healthcheck/HttpHealthCheckedEndpointGroupLongPollingTest.java
View file
Open in desktop
Expand Up @@ -52,12 +52,16 @@ class HttpHealthCheckedEndpointGroupLongPollingTest {
private static final String HEALTH_CHECK_PATH = "/healthcheck";

private static final SettableHealthChecker health = new SettableHealthChecker();
private static final Duration LONG_POLLING_TIMEOUT = Duration.ofSeconds(1);

@RegisterExtension
static final ServerExtension server = new ServerExtension() {
@Override
protected void configure(ServerBuilder sb) throws Exception {
sb.service(HEALTH_CHECK_PATH, HealthCheckService.of(health));
sb.service(HEALTH_CHECK_PATH, HealthCheckService.builder()
.longPolling(LONG_POLLING_TIMEOUT)
.checkers(health)
.build());

// Enable graceful shutdown so that the server is given a chance
// to send a health check response when the server is shutting down.
Expand All @@ -73,6 +77,7 @@ protected void configure(ServerBuilder sb) throws Exception {
@BeforeEach
void startServer() {
server.start();
healthCheckRequestLogs = null;
}

@Test
Expand Down Expand Up @@ -133,8 +138,6 @@ void longPollingDisabledOnStop() throws Exception {
healthCheckRequestLogs.take();
assertThat(stopwatch.elapsed(TimeUnit.MILLISECONDS))
.isGreaterThan(RETRY_INTERVAL.toMillis() * 4 / 5);
} finally {
this.healthCheckRequestLogs = null;
}
}

Expand Down Expand Up @@ -167,6 +170,40 @@ void periodicCheckWhenConnectionRefused() throws Exception {
}
}

@Test
@Timeout(15)
void keepEndpointHealthinessWhenLongPollingTimeout() throws Exception {
final BlockingQueue<RequestLog> healthCheckRequestLogs = new LinkedTransferQueue<>();
this.healthCheckRequestLogs = healthCheckRequestLogs;
final Endpoint endpoint = Endpoint.of("127.0.0.1", server.httpPort());
try (HealthCheckedEndpointGroup endpointGroup = build(
HealthCheckedEndpointGroup.builder(
new StaticEndpointGroup(endpoint),
HEALTH_CHECK_PATH))) {

// Check the initial state (healthy).
assertThat(endpointGroup.endpoints()).containsExactly(endpoint);

// Drop the first request.
healthCheckRequestLogs.take();

// Check the endpoint is populated even after long polling timeout.
Thread.sleep(LONG_POLLING_TIMEOUT.toMillis());
assertThat(endpointGroup.endpoints()).containsExactly(endpoint);

// Must receive the '304 Not Modified' response with long polling.
final ResponseHeaders notModifiedResponseHeaders = healthCheckRequestLogs.take().responseHeaders();
assertThat(notModifiedResponseHeaders.status()).isEqualTo(HttpStatus.NOT_MODIFIED);
assertThat(notModifiedResponseHeaders.getLong("armeria-lphc", 1))
.isEqualTo(LONG_POLLING_TIMEOUT.getSeconds());

final Stopwatch stopwatch = Stopwatch.createStarted();
healthCheckRequestLogs.take();
assertThat(stopwatch.elapsed(TimeUnit.MILLISECONDS))
.isLessThanOrEqualTo(LONG_POLLING_TIMEOUT.toMillis());
}
}

/**
* Makes sure the notification occurs as soon as possible thanks to long polling.
*/
Expand Down