[SPARK-27868][CORE][FOLLOWUP] Recover the default value to -1 again#27230
[SPARK-27868][CORE][FOLLOWUP] Recover the default value to -1 again#27230xCASx wants to merge 1 commit intoapache:masterfrom
Conversation
The default value for backLog set back to -1, as any other value may break existing configuration by overriding Netty's default io.netty.util.NetUtil#SOMAXCONN. The documentation accordingly adjusted. See discussion thread: apache#24732
|
ok to test |
|
Test build #116865 has finished for PR 27230 at commit
|
| /** Requested maximum length of the queue of incoming connections. Default is 64. */ | ||
| public int backLog() { return conf.getInt(SPARK_NETWORK_IO_BACKLOG_KEY, 64); } | ||
| /** | ||
| * Requested maximum length of the queue of incoming connections. If < 1, |
There was a problem hiding this comment.
@xCASx . < 1 looks a little strange. Can we revise?
There was a problem hiding this comment.
if (conf.backLog() > 0) {
bootstrap.option(ChannelOption.SO_BACKLOG, conf.backLog());
}I've been thinking about wording. If use 0 or negative instead of < 1 it may seem that separately mentioned 0 is some special case different to negative values.
For me it's not a big deal, if you'd like, I can change it to 0 or negative.
There was a problem hiding this comment.
What I meant was If < 1 looks strange grammatically to me.
| up with a large number of connections arriving in a short period of time. This needs to | ||
| be configured wherever the shuffle service itself is running, which may be outside of the | ||
| application (see <code>spark.shuffle.service.enabled</code> option below). | ||
| application (see <code>spark.shuffle.service.enabled</code> option below). If set below 1, |
There was a problem hiding this comment.
Just to be clear, do you mean 0 and negative values by If set below 1?
dongjoon-hyun
changed the title
dongjoon-hyun
changed the title
|
Hi, @vanzin . Could you review and finalize SPARK-27868 in the master branch please? |
|
Seems fine to me. I'd have worded it a little differently, but not a big deal. |
|
Merging to master. |
4 participants
The default value for backLog set back to -1, as any other value may break existing configuration by overriding Netty's default io.netty.util.NetUtil#SOMAXCONN. The documentation accordingly adjusted.
See discussion thread: #24732
What changes were proposed in this pull request?
Partial rollback of #24732 (default for backLog set back to -1).
Why are the changes needed?
Previous change introduces backward incompatibility by overriding default of Netty's
io.netty.util.NetUtil#SOMAXCONN