You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "georgew5656 (via GitHub)" <gi...@apache.org> on 2023/12/21 19:28:06 UTC
[PR] Allow overwriting ServerConnector accept queue size (druid)
georgew5656 opened a new pull request, #15596:
URL: https://github.com/apache/druid/pull/15596
Allow setting TCP accept queue size.
### Description
The ServerConnector class defaults _acceptQueueSize to 0 which normally results in a value of min(net.core.somaxconn, and whatever is passed to the listen() method on the socket). In my test environment with Java 11 this number is 50, which can be too low, especially when there are many brokers/coordinators trying to setup connections to a historical or real-time index process.
When this queue fills up there can be dropped packets and it is possible to see connection timeouts/connection resets depending on the underlying system settings.
This is a more advanced setting so we should continue setting the value of _acceptQueueSize to 0 when this config is not set.
#### Fixed the bug ...
#### Renamed the class ...
#### Added a forbidden-apis entry ...
I considered trying to set this to a fixed size but I think there are too many factors that go into what the correct size should be (including the type of server that is running, how many cpus are available, etc). So I think its best to leave this as a configurable option.
#### Release note
Add configuration for setting TCP acceptQueueSize.
##### Key changed/added classes in this PR
* `TLSServerConfig`
* `ServerConfig`
* `JettyServerModule`
This PR has:
- [X] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [X] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Increase ServerConnector accept queue size (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #15596:
URL: https://github.com/apache/druid/pull/15596#discussion_r1442469567
##########
server/src/main/java/org/apache/druid/server/initialization/jetty/JettyServerModule.java:
##########
@@ -491,6 +495,18 @@ private static int getMaxJettyAcceptorsSelectorsNum(DruidNode druidNode)
return numServerConnector * 8;
}
+ private static int getTCPAcceptQueueSize()
+ {
+ try {
+ BufferedReader in = Files.newBufferedReader(Paths.get("/proc/sys/net/core/somaxconn"));
+ return Integer.parseInt(in.readLine().trim());
+ }
+ catch (Exception e) {
+ log.warn("Unable to read /proc/sys/net/core/somaxconn, falling back to default value for TCP accept queue size");
Review Comment:
It'd be good to only log this warning on Linux. The file will never be there on macOS so the warning is always going to happen, which isn't ideal. Maybe use `System.getProperty("os.name")`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Increase ServerConnector accept queue size (druid)
Posted by "georgew5656 (via GitHub)" <gi...@apache.org>.
georgew5656 merged PR #15596:
URL: https://github.com/apache/druid/pull/15596
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org