Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/521#issuecomment-103145088
@HeartSaVioR,
I see you are right. There are two race conditions here:
1. If we do not check assignments in the read-lock, then we could use an
invalid connection. There could be an exception if we lose this race.
2. If we group by destination node+port when we equeue tuples for sending,
we lose the information needed to update the destination node+port when we
dequeue for sending, since assignments `task->node+port` can change in the
meantime. If we lose this race, then tuples for which there is a new
assignment may be dropped unnecessarily (if we fix 1.) or sent to the wrong
worker.
(Formerly, I did not realize 2. was also a regression with 861a92e, so I
was looking for a smaller change in this PR.)
> If you think latter is better to move on, I'll post a new PR based on
6ef2f11.
OK, I will take a look at your other branch.
> IMO it is a critical path for transfer efficiency, so it would be better
to have more reviewer for this patch.
I agree. The more, the better.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
