From 107b42f5b67312f6bf2baf1a2bdea3228be1db7b Mon Sep 17 00:00:00 2001 From: Cristian Osiac Date: Thu, 13 Jan 2022 17:41:20 +0000 Subject: [PATCH] Fix HTTP event listener retry delay calculation --- .../plugin/httpquery/HttpEventListener.java | 4 +++ .../httpquery/TestHttpQueryListener.java | 26 +++++++++---------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/plugin/trino-http-event-listener/src/main/java/io/trino/plugin/httpquery/HttpEventListener.java b/plugin/trino-http-event-listener/src/main/java/io/trino/plugin/httpquery/HttpEventListener.java index ebef2239d8bb..fb6a05ace45a 100644 --- a/plugin/trino-http-event-listener/src/main/java/io/trino/plugin/httpquery/HttpEventListener.java +++ b/plugin/trino-http-event-listener/src/main/java/io/trino/plugin/httpquery/HttpEventListener.java @@ -164,6 +164,10 @@ public void onFailure(Throwable t) private Duration nextDelay(Duration delay) { + if (delay.compareTo(Duration.valueOf("0s")) == 0) { + return config.getRetryDelay(); + } + Duration newDuration = Duration.succinctDuration(delay.getValue(TimeUnit.SECONDS) * this.config.getBackoffBase(), TimeUnit.SECONDS); if (newDuration.compareTo(config.getMaxDelay()) > 0) { return config.getMaxDelay(); diff --git a/plugin/trino-http-event-listener/src/test/java/io/trino/plugin/httpquery/TestHttpQueryListener.java b/plugin/trino-http-event-listener/src/test/java/io/trino/plugin/httpquery/TestHttpQueryListener.java index 5d5556a763ec..8a6262b91426 100644 --- a/plugin/trino-http-event-listener/src/test/java/io/trino/plugin/httpquery/TestHttpQueryListener.java +++ b/plugin/trino-http-event-listener/src/test/java/io/trino/plugin/httpquery/TestHttpQueryListener.java @@ -227,7 +227,7 @@ public void testAllLoggingDisabledShouldTimeout() querylogListener.queryCompleted(null); querylogListener.splitCompleted(null); - assertNull(server.takeRequest(1, TimeUnit.SECONDS)); + assertNull(server.takeRequest(5, TimeUnit.SECONDS)); } @Test @@ -246,13 +246,13 @@ public void testAllLoggingEnabledShouldSendCorrectEvent() server.enqueue(new MockResponse().setResponseCode(200)); querylogListener.queryCreated(queryCreatedEvent); - checkRequest(server.takeRequest(1, TimeUnit.SECONDS), queryCreatedEvent); + checkRequest(server.takeRequest(5, TimeUnit.SECONDS), queryCreatedEvent); querylogListener.queryCompleted(queryCompleteEvent); - checkRequest(server.takeRequest(1, TimeUnit.SECONDS), queryCompleteEvent); + checkRequest(server.takeRequest(5, TimeUnit.SECONDS), queryCompleteEvent); querylogListener.splitCompleted(splitCompleteEvent); - checkRequest(server.takeRequest(1, TimeUnit.SECONDS), splitCompleteEvent); + checkRequest(server.takeRequest(5, TimeUnit.SECONDS), splitCompleteEvent); } @Test @@ -268,7 +268,7 @@ public void testContentTypeDefaultHeaderShouldAlwaysBeSet() querylogListener.queryCompleted(queryCompleteEvent); - assertEquals(server.takeRequest(1, TimeUnit.SECONDS).getHeader("Content-Type"), "application/json; charset=utf-8"); + assertEquals(server.takeRequest(5, TimeUnit.SECONDS).getHeader("Content-Type"), "application/json; charset=utf-8"); } public void testHttpHeadersShouldBePresent() @@ -284,7 +284,7 @@ public void testHttpHeadersShouldBePresent() querylogListener.queryCompleted(queryCompleteEvent); - checkRequest(server.takeRequest(1, TimeUnit.SECONDS), new HashMap<>() {{ + checkRequest(server.takeRequest(5, TimeUnit.SECONDS), new HashMap<>() {{ put("Authorization", "Trust Me!"); put("Cache-Control", "no-cache"); }}, queryCompleteEvent); @@ -305,7 +305,7 @@ public void testHttpsEnabledShouldUseTLSv13() }}); querylogListener.queryCompleted(queryCompleteEvent); - RecordedRequest recordedRequest = server.takeRequest(1, TimeUnit.SECONDS); + RecordedRequest recordedRequest = server.takeRequest(5, TimeUnit.SECONDS); assertNotNull(recordedRequest, "Handshake probably failed"); assertEquals(recordedRequest.getTlsVersion().javaName(), "TLSv1.3"); @@ -328,7 +328,7 @@ public void testDifferentCertificatesShouldNotSendRequest() }}); querylogListener.queryCompleted(queryCompleteEvent); - RecordedRequest recordedRequest = server.takeRequest(1, TimeUnit.SECONDS); + RecordedRequest recordedRequest = server.takeRequest(5, TimeUnit.SECONDS); assertNull(recordedRequest, "Handshake should have failed"); } @@ -347,7 +347,7 @@ public void testNoServerCertificateShouldNotSendRequest() }}); querylogListener.queryCompleted(queryCompleteEvent); - RecordedRequest recordedRequest = server.takeRequest(1, TimeUnit.SECONDS); + RecordedRequest recordedRequest = server.takeRequest(5, TimeUnit.SECONDS); assertNull(recordedRequest, "Handshake should have failed"); } @@ -367,8 +367,8 @@ public void testServer500ShouldRetry() querylogListener.queryCompleted(queryCompleteEvent); - assertNotNull(server.takeRequest(1, TimeUnit.SECONDS)); // First request that responds with 500 - checkRequest(server.takeRequest(1, TimeUnit.SECONDS), queryCompleteEvent); // The retry that responds with 200 + assertNotNull(server.takeRequest(5, TimeUnit.SECONDS)); // First request that responds with 500 + checkRequest(server.takeRequest(5, TimeUnit.SECONDS), queryCompleteEvent); // The retry that responds with 200 } @Test @@ -386,8 +386,8 @@ public void testServer400ShouldRetry() querylogListener.queryCompleted(queryCompleteEvent); - assertNotNull(server.takeRequest(1, TimeUnit.SECONDS)); // First request, send back 400 - checkRequest(server.takeRequest(1, TimeUnit.SECONDS), queryCompleteEvent); // The retry that responds with 200 + assertNotNull(server.takeRequest(5, TimeUnit.SECONDS)); // First request, send back 400 + checkRequest(server.takeRequest(5, TimeUnit.SECONDS), queryCompleteEvent); // The retry that responds with 200 } @Test