[GitHub] jon-wei commented on a change in pull request #6552: Use current coordinator leader instead of cached one (#6551)

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org