jon-wei commented on a change in pull request #6552: Use current coordinator
leader instead of cached one (#6551)
URL: https://github.com/apache/incubator-druid/pull/6552#discussion_r230517978
##########
File path:
server/src/main/java/org/apache/druid/discovery/DruidLeaderClient.java
##########
@@ -122,11 +122,21 @@ public void stop()
/**
* Make a Request object aimed at the leader. Throws IOException if the
leader cannot be located.
+ *
+ * @param cached Uses cached leader if true, else uses the current leader
*/
- public Request makeRequest(HttpMethod httpMethod, String urlPath) throws
IOException
+ public Request makeRequest(HttpMethod httpMethod, String urlPath, boolean
cached) throws IOException
{
Preconditions.checkState(lifecycleLock.awaitStarted(1,
TimeUnit.MILLISECONDS));
- return new Request(httpMethod, new URL(StringUtils.format("%s%s",
getCurrentKnownLeader(true), urlPath)));
+ return new Request(httpMethod, new URL(StringUtils.format("%s%s",
getCurrentKnownLeader(cached), urlPath)));
Review comment:
Mentioning here that the "cache" parameter may not really be necessary,
since `pickOneHost` appears to be reading cached state already from
`serverDiscoverySelector` or `druidNodeDiscovery`.
The other callers of makeRequest (with caching true) call
DruidLeaderClient.go() which will retry with getCurrentKnownLeader(false) on
failures, so they weren't affected by this stale leader issue.
Given that `makeRequest` is used by multiple callers though and we've
already cut 0.13.0, I don't think we need to risk changing that behavior (i.e.,
does getCurrentKnownLeader really needs its own caching) for now though.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
