maedhroz commented on code in PR #4902:
URL: https://github.com/apache/cassandra/pull/4902#discussion_r3483113163
##########
src/java/org/apache/cassandra/concurrent/SharedExecutorPool.java:
##########
@@ -123,7 +123,8 @@ void workerEnded(SEPWorker worker)
public Stream<? extends DebuggableTaskRunner> workers()
{
- return allWorkers.stream();
+ return Stream.concat(allWorkers.stream(),
+
allWorkers.stream().map(SEPWorker::immediateRunner));
Review Comment:
From CC shallow review:
> Finding 7: Stream.concat(allWorkers.stream(),
allWorkers.stream().map(...)) iterates the concurrent set twice — snapshot
inconsistency on worker churn
>
> - Location:
src/java/org/apache/cassandra/concurrent/SharedExecutorPool.java:125-127
> - Confidence: Medium
> - Flagged by: Concurrency, Resources
> - What's wrong: allWorkers is Collections.newSetFromMap(new
ConcurrentHashMap<>()). The two allWorkers.stream() calls produce two
independent iterations; between them, workers can be added/removed by
schedule() / workerEnded(). The two streams can yield different worker sets —
main row
> without immediate row for a freshly-removed worker, immediate-only row
for one added between the two traversals. Also doubles the iteration cost on
each SELECT * FROM system_views.queries even at idle.
> - Suggested fix:
> return allWorkers.stream().flatMap(w -> Stream.of((DebuggableTaskRunner)
w, w.immediateRunner()));
--
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]
