abhishekrb19 commented on code in PR #19448:
URL: https://github.com/apache/druid/pull/19448#discussion_r3229000137


##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/WorkerHolder.java:
##########
@@ -65,8 +65,9 @@ public class WorkerHolder
   public static final TypeReference<ChangeRequestsSnapshot<WorkerHistoryItem>> 
WORKER_SYNC_RESP_TYPE_REF = new TypeReference<>() {};
 
 
-  private final Worker worker;
-  private Worker disabledWorker;
+  // Pre-built views with isDisabled() set to false/true respectively. 
getWorker() selects between them based on `disabled`.

Review Comment:
   nit: make this a javadoc instead
   ```suggestion
     /**
      *Pre-built views with isDisabled() set to false/true respectively. {@link 
#getWorker()} selects between them based on {@link #disabled}.
      */
   ```



##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/WorkerHolder.java:
##########
@@ -145,23 +147,8 @@ public void setBlacklistedUntil(DateTime blacklistedUntil)
 
   public ImmutableWorkerInfo toImmutable()
   {
-    Worker w = worker;
-    if (disabled.get()) {
-      if (disabledWorker == null) {
-        disabledWorker = new Worker(
-            worker.getScheme(),
-            worker.getHost(),
-            worker.getIp(),
-            worker.getCapacity(),
-            "",

Review Comment:
   This is where version was set to "" previously, not sure why 
`worker.getVersion()` wasn't propagated all the way through in the first place



##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/ZkWorker.java:
##########
@@ -205,7 +205,8 @@ public boolean isRunningTask(String taskId)
   @UsedInGeneratedCode // See JavaScriptWorkerSelectStrategyTest
   public boolean isValidVersion(String minVersion)
   {
-    return worker.get().getVersion().compareTo(minVersion) >= 0;
+    var w = worker.get();

Review Comment:
   Are `var`s used now with the target JDK bumped? It seems like some tests do 
but non-test main code still use strongly typed variables. So I think it'd 
probably be best to align with that code style for now.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to