jasonk000 commented on code in PR #13982:
URL: https://github.com/apache/druid/pull/13982#discussion_r1192923573
##########
server/src/main/java/org/apache/druid/segment/realtime/appenderator/AppenderatorImpl.java:
##########
@@ -1146,9 +1160,9 @@ private void initializeExecutors()
if (persistExecutor == null) {
// use a blocking single threaded executor to throttle the firehose when
write to disk is slow
persistExecutor = MoreExecutors.listeningDecorator(
- Execs.newBlockingSingleThreaded(
+ Execs.newBlockingThreaded(
"[" + StringUtils.encodeForFormat(myId) +
"]-appenderator-persist",
- maxPendingPersists
+ persistThreads, maxPendingPersists
Review Comment:
I'm wary of the `persistExecutor` being called by callers assuming it is
single-threaded. For example, there's nothing I can see that prevents the
`close()` from occurring while a task is still running. Nothing stands out as
obviously broken though. But, I do see comments like this:
```
// use persistExecutor to make sure that all the pending persists
completes before
// starting to abandon segments
persistExecutor
```
Comments suggest that - if we have multiple threads - we might start
processing an abandon request before or while a persist request is in queue.
What scenarios/mitigations are there here?
--
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]
