diff options
author | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2022-12-08 12:14:26 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-08 12:14:26 +0100 |
commit | 2a7f0fb650a57a62983e6ee64f29fbfe576366c9 (patch) | |
tree | b02c0c47829b37ab3a372c5a0de9da9a1513e2a9 | |
parent | dc85b21d427069b982d8e0888c0a4062e5491fb1 (diff) | |
parent | 95dd8d9ef4b4ef279a2dac044c41e613dbb20caf (diff) |
Merge pull request #25167 from vespa-engine/jonmv/feed-client-handshake
Handle old servers in handshake
4 files changed, 58 insertions, 17 deletions
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java index c70cb7cd850..bc801dabf2d 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java @@ -172,7 +172,7 @@ public class FeedClientBuilderImpl implements FeedClientBuilder { } @Override - public FeedClientBuilder setSpeedTest(boolean enabled) { + public FeedClientBuilderImpl setSpeedTest(boolean enabled) { this.speedTest = enabled; return this; } @@ -194,7 +194,11 @@ public class FeedClientBuilderImpl implements FeedClientBuilder { return this; } - @Override public FeedClientBuilder setProxy(URI uri) { this.proxy = uri; return this; } + @Override + public FeedClientBuilderImpl setProxy(URI uri) { + this.proxy = uri; + return this; + } /** Constructs instance of {@link ai.vespa.feed.client.FeedClient} from builder configuration */ @Override diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java index 2427b71a104..cad2da4d242 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpFeedClient.java @@ -51,14 +51,18 @@ class HttpFeedClient implements FeedClient { private final boolean speedTest; HttpFeedClient(FeedClientBuilderImpl builder) throws IOException { - this(builder, new HttpRequestStrategy(builder)); + this(builder, builder.dryrun ? new DryrunCluster() : new ApacheCluster(builder)); } - HttpFeedClient(FeedClientBuilderImpl builder, RequestStrategy requestStrategy) { + HttpFeedClient(FeedClientBuilderImpl builder, Cluster cluster) { + this(builder, cluster, new HttpRequestStrategy(builder, cluster)); + } + + HttpFeedClient(FeedClientBuilderImpl builder, Cluster cluster, RequestStrategy requestStrategy) { this.requestHeaders = new HashMap<>(builder.requestHeaders); this.requestStrategy = requestStrategy; this.speedTest = builder.speedTest; - verifyConnection(builder); + verifyConnection(builder, cluster); } @Override @@ -121,9 +125,8 @@ class HttpFeedClient implements FeedClient { return promise; } - private void verifyConnection(FeedClientBuilderImpl builder) { - if (builder.dryrun) return; - try (Cluster cluster = new ApacheCluster(builder)) { + private void verifyConnection(FeedClientBuilderImpl builder, Cluster cluster) { + try { HttpRequest request = new HttpRequest("POST", getPath(DocumentId.of("feeder", "handshake", "dummy")) + getQuery(empty(), true), requestHeaders, @@ -140,12 +143,15 @@ class HttpFeedClient implements FeedClient { default: message = response.toString(); break; } else message = response.toString(); + + // Old server ignores ?dryRun=true, but getting this particular error message means everything else is OK. + if (response.code() == 400 && "Could not read document, no document?".equals(message)) { + if (builder.speedTest) throw new FeedException("server does not support speed test; upgrade to a newer version"); + return; + } throw new FeedException("server responded non-OK to handshake: " + message); } } - catch (IOException e) { - throw new FeedException("failed initiating handshake with server: " + e, e); - } catch (ExecutionException e) { throw new FeedException("failed handshake with server: " + e.getCause(), e.getCause()); } diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java index 2d052e8a6af..ce86ad59ffe 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/HttpRequestStrategy.java @@ -68,10 +68,6 @@ class HttpRequestStrategy implements RequestStrategy { return thread; }); - HttpRequestStrategy(FeedClientBuilderImpl builder) throws IOException { - this(builder, builder.dryrun ? new DryrunCluster() : new ApacheCluster(builder)); - } - HttpRequestStrategy(FeedClientBuilderImpl builder, Cluster cluster) { this.cluster = builder.benchmark ? new BenchmarkingCluster(cluster) : cluster; this.strategy = builder.retryStrategy; diff --git a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpFeedClientTest.java b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpFeedClientTest.java index a0b3049c2a0..295108b5ed5 100644 --- a/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpFeedClientTest.java +++ b/vespa-feed-client/src/test/java/ai/vespa/feed/client/impl/HttpFeedClientTest.java @@ -3,7 +3,6 @@ package ai.vespa.feed.client.impl; import ai.vespa.feed.client.DocumentId; import ai.vespa.feed.client.FeedClient; -import ai.vespa.feed.client.FeedClientBuilder; import ai.vespa.feed.client.FeedException; import ai.vespa.feed.client.HttpResponse; import ai.vespa.feed.client.OperationParameters; @@ -44,6 +43,7 @@ class HttpFeedClientTest { @Override public CompletableFuture<HttpResponse> enqueue(DocumentId documentId, HttpRequest request) { return dispatch.get().apply(documentId, request); } } FeedClient client = new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))).setDryrun(true), + new DryrunCluster(), new MockRequestStrategy()); // Update is a PUT, and 200 OK is a success. @@ -212,9 +212,44 @@ class HttpFeedClientTest { @Test void testHandshake() { + // dummy:123 does not exist, and results in a host-not-found exception. assertTrue(assertThrows(FeedException.class, - () -> new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))), null)) + () -> new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))))) .getMessage().startsWith("failed handshake with server: java.net.UnknownHostException")); + + HttpResponse oldResponse = HttpResponse.of(400, "{\"pathId\":\"/document/v1/test/build/docid/foo\",\"message\":\"Could not read document, no document?\"}".getBytes(UTF_8)); + HttpResponse okResponse = HttpResponse.of(200, null); + AtomicReference<HttpResponse> response = new AtomicReference<>(oldResponse); + Cluster cluster = (request, vessel) -> { + try { + assertNull(request.body()); + assertEquals("POST", request.method()); + assertEquals("/document/v1/feeder/handshake/docid/dummy?dryRun=true", request.path()); + vessel.complete(response.get()); + } + catch (Throwable t) { + vessel.completeExceptionally(t); + } + }; + + // Old server, and speed-test. + assertEquals("server does not support speed test; upgrade to a newer version", + assertThrows(FeedException.class, + () -> new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))).setSpeedTest(true), + cluster, + null)) + .getMessage()); + + // Old server. + new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))), + cluster, + null); + + // New server. + response.set(okResponse); + new HttpFeedClient(new FeedClientBuilderImpl(Collections.singletonList(URI.create("https://dummy:123"))), + cluster, + null); } } |