kfaraz commented on PR #14643:
URL: https://github.com/apache/druid/pull/14643#issuecomment-1647197115
I feel that re-introducing locks is a step backwards. If the changes in
#14435 seem to be causing trouble, we should just revert that commit rather
than introduce a new kind of locking. This would also make sense given the
upcoming release of Druid 27.
---
**Alternatively,**
IIUC, the problem here is that while one thread is in the middle of adding
the work item using `tasks.computeIfAbsent`, the executor has already picked it
up for running.
There are two easy ways to avoid that (unless I am missing something):
#### Option 1: Set `result` in `KubernetesWorkItem` only after work item has
been added to map:
```java
@Override
public ListenableFuture<TaskStatus> run(Task task)
{
final KubernetesWorkItem workItem = tasks.computeIfAbsent(
task.getId(), k -> new KubernetesWorkItem(task, exec.submit(() ->
runTask(task)))
);
workItem.setResultIfRequired(
exec.submit(() -> runTask(task))
);
return workItem.getResult();
}
```
This requires creating a new method `synchronized void
setResultIfRequired()` inside `KubernetesWorkItem`.
Something similar would need to be done for `joinAsync` method too.
#### Option 2: Start `doTask` only after `computeIfAbsent` has finished.
```java
private TaskStatus runTask(Task task)
{
final AtomicReference<TaskStatus> taskStatus = new AtomicReference<>();
tasks.compute(
task.getId(), (taskId, workItem) -> {
taskStatus.set(doTask(task, workItem, true));
return workItem;
}
);
return taskStatus.get();
}
```
Something similar would have to be done for `joinTask` method too.
---
I personally prefer option 1 as it doesn't unncessarily block an executor
thread and is logically simpler to follow.
@YongGang , @georgew5656 , what do you think?
--
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]
