[GitHub] [incubator-druid] jihoonson commented on issue #8697: HRTR: make pending task execution handling to go through all tasks on not finding worker slots

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

jihoonson commented on issue #8697: HRTR: make pending task execution handling to go through all tasks on not finding worker slots
URL: https://github.com/apache/incubator-druid/pull/8697#issuecomment-564316196
 
 
   > lgtm issue can be ignored in this case and I have been running this patch in prod for quite some time. in any case, we are probably only one using HRTR for now.
   
   Hi @himanshug, I'm reviewing this PR. The LGTM warning is about the below code block.
   
   ```java
           try {
             if (immutableWorker == null) {
               throw new ISE("NULL immutableWorker");
             }
   
             // this will send HTTP request to worker for assigning task
             if (!runTaskOnWorker(taskItem, immutableWorker.getWorker().getHost())) {
               if (taskItem.getState() == HttpRemoteTaskRunnerWorkItem.State.PENDING_WORKER_ASSIGN) {
                 taskItem.revertStateFromPendingWorkerAssignToPending();
               }
             }
           }
           catch (InterruptedException ex) {
             log.info("Got InterruptedException while assigning task[%s].", taskId);
             throw ex;
           }
           catch (Throwable th) {
             log.makeAlert(th, "Exception while trying to assign task")
                .addData("taskId", taskId)
                .emit();
   
             taskComplete(taskItem, null, TaskStatus.failure(taskId));
           }
           finally {
             synchronized (statusLock) {
               workersWithUnacknowledgedTask.remove(immutableWorker.getWorker().getHost());
               statusLock.notifyAll();
             }
           }
   ```
   
   It seems like `immutableWorker` can be null in the `finally` clause and so the LGTM warning looks legit to me. Would you explain more why this is ok and can be ignored?

----------------------------------------------------------------
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.
 
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