-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-4740] Create multiple concurrent connections between two peer nodes in Netty. #3625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Test build #24193 has started for PR 3625 at commit
|
|
Test build #24193 has finished for PR 3625 at commit
|
|
Test FAILed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can make this a private static class if we make this a constructor parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 spaces for indent
|
Test build #24198 has started for PR 3625 at commit
|
|
Looks mostly good to me, a few remaining synchronization issues. Will take another long look after you address all comments. I'd really appreciate a test, though, if we can get one in -- we really don't want to be regressing at this point, and we also really want to make sure we're fixing the issue. |
|
Test build #24199 has started for PR 3625 at commit
|
|
Test build #24198 has finished for PR 3625 at commit
|
|
Test FAILed. |
|
Test build #24200 has started for PR 3625 at commit
|
|
Test build #24199 has finished for PR 3625 at commit
|
|
Test PASSed. |
|
Test build #24200 has finished for PR 3625 at commit
|
|
Test PASSed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Can we use JavaUtils.closeQuietly(client) here?
|
Test build #24206 has started for PR 3625 at commit
|
|
Test FAILed. |
|
LGTM |
|
Jenkins, retest this please. |
|
Test build #24207 has started for PR 3625 at commit
|
|
Test build #24206 has finished for PR 3625 at commit
|
|
Test PASSed. |
|
Test build #24207 has finished for PR 3625 at commit
|
|
Test PASSed. |
|
I'm merging this one in master & branch-1.2. |
…nodes in Netty. It's been reported that when the number of disks is large and the number of nodes is small, Netty network throughput is low compared with NIO. We suspect the problem is that only a small number of disks are utilized to serve shuffle files at any given point, due to connection reuse. This patch adds a new config parameter to specify the number of concurrent connections between two peer nodes, default to 2. Author: Reynold Xin <[email protected]> Closes #3625 from rxin/SPARK-4740 and squashes the following commits: ad4241a [Reynold Xin] Updated javadoc. f33c72b [Reynold Xin] Code review feedback. 0fefabb [Reynold Xin] Use double check in synchronization. 41dfcb2 [Reynold Xin] Added test case. 9076b4a [Reynold Xin] Fixed two NPEs. 3e1306c [Reynold Xin] Minor style fix. 4f21673 [Reynold Xin] [SPARK-4740] Create multiple concurrent connections between two peer nodes in Netty. (cherry picked from commit 2b9b726) Signed-off-by: Reynold Xin <[email protected]>
It's been reported that when the number of disks is large and the number of nodes is small, Netty network throughput is low compared with NIO. We suspect the problem is that only a small number of disks are utilized to serve shuffle files at any given point, due to connection reuse. This patch adds a new config parameter to specify the number of concurrent connections between two peer nodes, default to 2.