[
https://issues.apache.org/jira/browse/STORM-737?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14548377#comment-14548377
]
ASF GitHub Bot commented on STORM-737:
--------------------------------------
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.
> Workers may try to send to closed connections
> ---------------------------------------------
>
> Key: STORM-737
> URL: https://issues.apache.org/jira/browse/STORM-737
> Project: Apache Storm
> Issue Type: Bug
> Affects Versions: 0.9.2-incubating
> Reporter: Derek Dagit
>
> There is a race condition in the worker code that can allow for a send() to
> be called on a closed connection.
> [Discussion|https://github.com/apache/storm/pull/349#issuecomment-87778672]
> The assignment mapping from task -> node+port needs to be read and used in
> the read lock when sending, so that an accurate mapping is used that does not
> include any connections that are closed.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
