raulpardo commented on code in PR #26219:
URL: https://github.com/apache/flink/pull/26219#discussion_r1976574603
##########
flink-runtime/src/main/java/org/apache/flink/streaming/runtime/tasks/mailbox/TaskMailboxImpl.java:
##########
@@ -243,11 +243,11 @@ private Mail takeOrNull(Deque<Mail> queue, int priority) {
@Override
public List<Mail> drain() {
- List<Mail> drainedMails = new ArrayList<>(batch);
- batch.clear();
Review Comment:
This is a good idea that would solve all data races. However, I noticed that
the current implementation seems to be acquiring locks depending on what thread
is executing the method by invoking `checkIsMailboxThread()`. It might be that
this was introduced for performance reasons; as acquiring the lock is a rather
slow operation. But this is not entirely clear to me, as there are other
methods like `createBatch` that acquire the lock even though the first
instruction is to check is to ensure that only the mailbox thread is executing
this.
This is why I reported only the data race on the `drain` method, as there is
no instruction indicating that only the mailbox thread can execute that method.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
