This is an automated email from the ASF dual-hosted git repository.
abhishek pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 48f4330100 Make leader redirection work when both plainText and TLS
ports are set (#13847)
48f4330100 is described below
commit 48f4330100e73be5dbf85c3002eaf0d4e5224e63
Author: Abhishek Agarwal <[email protected]>
AuthorDate: Sun Feb 26 21:23:29 2023 +0530
Make leader redirection work when both plainText and TLS ports are set
(#13847)
When both plainText and TLS ports are set in druid, the redirection to a
different leader node can fail. This is caused by how we compare a redirect
path and the leader locations registered with a druid node. While the
registered location has both plainText and TLS port set, the redirect path only
has one port since it's a URI.
---
.../java/org/apache/druid/rpc/ServiceClientImpl.java | 19 +++++++++++++------
.../org/apache/druid/rpc/ServiceClientImplTest.java | 20 ++++++++++++++++++++
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/server/src/main/java/org/apache/druid/rpc/ServiceClientImpl.java
b/server/src/main/java/org/apache/druid/rpc/ServiceClientImpl.java
index eca2cfdc5a..98191b3e13 100644
--- a/server/src/main/java/org/apache/druid/rpc/ServiceClientImpl.java
+++ b/server/src/main/java/org/apache/druid/rpc/ServiceClientImpl.java
@@ -301,8 +301,7 @@ public class ServiceClientImpl implements ServiceClient
);
} else if (serviceLocations.getLocations()
.stream()
- .anyMatch(loc ->
serviceLocationNoPath(loc)
-
.equals(redirectLocationNoPath))) {
+ .anyMatch(loc ->
serviceLocationMatches(loc, redirectLocationNoPath))) {
// Valid redirect, to a server that is one of the known
locations.
final boolean isRedirectLoop =
redirectLocations.contains(newUri);
final boolean isRedirectChainTooLong =
redirectLocations.size() >= MAX_REDIRECTS;
@@ -415,7 +414,7 @@ public class ServiceClientImpl implements ServiceClient
if (preferred != null) {
// Preferred location is set. Use it if it's one of known locations.
for (final ServiceLocation location : locations.getLocations()) {
- if (serviceLocationNoPath(location).equals(preferred)) {
+ if (serviceLocationMatches(location, preferred)) {
return location;
}
}
@@ -511,11 +510,19 @@ public class ServiceClientImpl implements ServiceClient
}
/**
- * Returns a {@link ServiceLocation} without its path.
+ * Returns true if two service locations are same or false otherwise. If a
port is negative, we ignore that
+ * port for comparison.
*/
- static ServiceLocation serviceLocationNoPath(final ServiceLocation location)
+ static boolean serviceLocationMatches(final ServiceLocation left, final
ServiceLocation right)
{
- return new ServiceLocation(location.getHost(),
location.getPlaintextPort(), location.getTlsPort(), "");
+ return left.getHost().equals(right.getHost())
+ && portMatches(left.getPlaintextPort(), right.getPlaintextPort())
+ && portMatches(left.getTlsPort(), right.getTlsPort());
+ }
+
+ static boolean portMatches(int left, int right)
+ {
+ return left < 0 || right < 0 || left == right;
}
@VisibleForTesting
diff --git
a/server/src/test/java/org/apache/druid/rpc/ServiceClientImplTest.java
b/server/src/test/java/org/apache/druid/rpc/ServiceClientImplTest.java
index dc8bba87d1..20487aeac8 100644
--- a/server/src/test/java/org/apache/druid/rpc/ServiceClientImplTest.java
+++ b/server/src/test/java/org/apache/druid/rpc/ServiceClientImplTest.java
@@ -70,6 +70,8 @@ public class ServiceClientImplTest
private static final ServiceLocation SERVER3 = new
ServiceLocation("example.com", -1, 1111, "/q");
private static final ServiceLocation SERVER4 = new
ServiceLocation("example.com", -1, 2222, "/q");
private static final ServiceLocation SERVER5 = new
ServiceLocation("example.com", -1, 3333, "/q");
+ private static final ServiceLocation SERVER6 = new
ServiceLocation("mixed.com", 201, 111, "/q");
+ private static final ServiceLocation SERVER7 = new
ServiceLocation("mixed.com", 203, 222, "/q");
private ScheduledExecutorService exec;
@@ -272,6 +274,24 @@ public class ServiceClientImplTest
Assert.assertEquals(expectedResponseObject, response);
}
+ @Test
+ public void test_request_followRedirect_mixedPorts() throws Exception
+ {
+ final RequestBuilder requestBuilder = new RequestBuilder(HttpMethod.GET,
"/foo");
+ final ImmutableMap<String, String> expectedResponseObject =
ImmutableMap.of("foo", "bar");
+
+ // Redirect from SERVER6 -> SERVER7.
+ stubLocatorCall(locations(SERVER6, SERVER7));
+ expectHttpCall(requestBuilder, SERVER6)
+
.thenReturn(redirectResponse(requestBuilder.build(SERVER7).getUrl().toString()));
+ expectHttpCall(requestBuilder,
SERVER7).thenReturn(valueResponse(expectedResponseObject));
+
+ serviceClient = makeServiceClient(StandardRetryPolicy.noRetries());
+ final Map<String, String> response = doRequest(serviceClient,
requestBuilder);
+
+ Assert.assertEquals(expectedResponseObject, response);
+ }
+
@Test
public void test_request_tooLongRedirectChain()
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
