You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/12/19 08:43:43 UTC
[GitHub] [pinot] ankitsultana opened a new pull request, #10001: Add Support for Tuning HTTP Server Thread Pool
ankitsultana opened a new pull request, #10001:
URL: https://github.com/apache/pinot/pull/10001
**Context:** At present the thread pool used by the http-server uses maxPoolSize = corePoolSize = Runtime.getRuntime().availableProcessors() * 2.
We need to tune the thread pool in some cases when the work load is not CPU bound (e.g. in controller).
**Test Plan**: Added a UT. Also tested on local by starting a controller and setting the config in controller.conf.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat merged pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
chenboat merged PR #10001:
URL: https://github.com/apache/pinot/pull/10001
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #10001:
URL: https://github.com/apache/pinot/pull/10001#issuecomment-1358053407
# [Codecov](https://codecov.io/gh/apache/pinot/pull/10001?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#10001](https://codecov.io/gh/apache/pinot/pull/10001?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b0d8643) into [master](https://codecov.io/gh/apache/pinot/commit/7eba70c0ed8159ecf2bd6b152a2d0374916cc068?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7eba70c) will **decrease** coverage by `37.74%`.
> The diff coverage is `71.79%`.
```diff
@@ Coverage Diff @@
## master #10001 +/- ##
=============================================
- Coverage 70.42% 32.68% -37.75%
+ Complexity 5662 200 -5462
=============================================
Files 1991 1992 +1
Lines 107251 107319 +68
Branches 16302 16317 +15
=============================================
- Hits 75533 35072 -40461
- Misses 26444 69196 +42752
+ Partials 5274 3051 -2223
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `24.47% <71.79%> (+0.07%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `13.60% <0.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/10001?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...org/apache/pinot/core/util/ListenerConfigUtil.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0xpc3RlbmVyQ29uZmlnVXRpbC5qYXZh) | `74.19% <66.66%> (-3.91%)` | :arrow_down: |
| [...e/pinot/core/transport/ServerThreadPoolConfig.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvU2VydmVyVGhyZWFkUG9vbENvbmZpZy5qYXZh) | `73.33% <73.33%> (ø)` | |
| [...rg/apache/pinot/core/transport/ListenerConfig.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvTGlzdGVuZXJDb25maWcuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...ain/java/org/apache/pinot/spi/utils/LoopUtils.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvTG9vcFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/user/RoleType.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3VzZXIvUm9sZVR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1231 more](https://codecov.io/gh/apache/pinot/pull/10001/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a diff in pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
chenboat commented on code in PR #10001:
URL: https://github.com/apache/pinot/pull/10001#discussion_r1052764512
##########
pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java:
##########
@@ -60,6 +61,7 @@
public final class ListenerConfigUtil {
private static final String DEFAULT_HOST = "0.0.0.0";
private static final String DOT_ACCESS_PROTOCOLS = ".access.protocols";
+ private static final String DOT_ACCESS_THREAD_POOL = ".thread.pool";
Review Comment:
Can you add in the PR summary section on what new config key(s) user should use to config the thread pool size?
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a diff in pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
chenboat commented on code in PR #10001:
URL: https://github.com/apache/pinot/pull/10001#discussion_r1054794176
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/ServerThreadPoolConfig.java:
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.transport;
+
+public class ServerThreadPoolConfig {
Review Comment:
Can you rename controller.thread.pool.corePoolSize to controller.httpserver.thread.pool.corePoolSize because it is about http servers running on controller/broker/server?
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on PR #10001:
URL: https://github.com/apache/pinot/pull/10001#issuecomment-1370291670
Can you help update the pinot doc about these new configs?
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] ankitsultana commented on a diff in pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
ankitsultana commented on code in PR #10001:
URL: https://github.com/apache/pinot/pull/10001#discussion_r1055088896
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/ServerThreadPoolConfig.java:
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.transport;
+
+public class ServerThreadPoolConfig {
Review Comment:
Good point. Updated to `controller.http.server.thread.pool.corePoolSize`.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] chenboat commented on a diff in pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
chenboat commented on code in PR #10001:
URL: https://github.com/apache/pinot/pull/10001#discussion_r1052763886
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/ServerThreadPoolConfig.java:
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.transport;
+
+public class ServerThreadPoolConfig {
Review Comment:
1) This is just for the threads for the http server used in server/broker/controller. Can we make the class name more explicit on this?
2) Add javadoc
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on PR #10001:
URL: https://github.com/apache/pinot/pull/10001#issuecomment-1371367160
Thanks! The doc is backed by this repo: https://github.com/pinot-contrib/pinot-docs
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] ankitsultana commented on a diff in pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
ankitsultana commented on code in PR #10001:
URL: https://github.com/apache/pinot/pull/10001#discussion_r1052805801
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/ServerThreadPoolConfig.java:
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.transport;
+
+public class ServerThreadPoolConfig {
Review Comment:
Renamed to `HttpServerThreadPoolConfig` and added a javadoc. Lmk your thoughts.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] ankitsultana commented on a diff in pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
ankitsultana commented on code in PR #10001:
URL: https://github.com/apache/pinot/pull/10001#discussion_r1052803551
##########
pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java:
##########
@@ -60,6 +61,7 @@
public final class ListenerConfigUtil {
private static final String DEFAULT_HOST = "0.0.0.0";
private static final String DOT_ACCESS_PROTOCOLS = ".access.protocols";
+ private static final String DOT_ACCESS_THREAD_POOL = ".thread.pool";
Review Comment:
Good point. Done.
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] ankitsultana commented on pull request #10001: Add Support for Tuning HTTP Server Thread Pool
Posted by GitBox <gi...@apache.org>.
ankitsultana commented on PR #10001:
URL: https://github.com/apache/pinot/pull/10001#issuecomment-1370465944
> Can you help update the pinot doc about these new configs?
Yup sure. @Jackie-Jiang I haven't done that before, can you share how I can do that?
--
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@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org