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. ---