You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "klsince (via GitHub)" <gi...@apache.org> on 2023/03/02 06:43:31 UTC
[GitHub] [pinot] klsince opened a new pull request, #10369: allow to customize the httpclient used to create S3PinotFS
klsince opened a new pull request, #10369:
URL: https://github.com/apache/pinot/pull/10369
This PR exposes some configs to customize the httpclient used to create s3client, when to init the S3PinotFS.
## Release Note ##
the exposed httpclient configs are
`httpclient.maxConnections`, e.g. 100 (this is more required for now. when we increased helixExecutorTask thread number via STATE_TRANSITION.maxThreads to load segments with more threads, some threads threw exceptions about timeout to get a http conn to download raw segments from deep store)
`httpclient.socketTimeout`, e.g. PT10S
`httpclient.connectionTimeout`, e.g. PT20S
`httpclient.connectionTimeToLive`, e.g. T30S
`httpclient.connectionAcquisitionTimeout`, e.g. PT40S
--
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 merged pull request #10369: allow to customize the httpclient used to create S3PinotFS
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #10369:
URL: https://github.com/apache/pinot/pull/10369
--
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] npawar commented on a diff in pull request #10369: allow to customize the httpclient used to create S3PinotFS
Posted by "npawar (via GitHub)" <gi...@apache.org>.
npawar commented on code in PR #10369:
URL: https://github.com/apache/pinot/pull/10369#discussion_r1124074878
##########
pinot-plugins/pinot-file-system/pinot-s3/src/main/java/org/apache/pinot/plugin/filesystem/S3Config.java:
##########
@@ -111,6 +125,54 @@ public S3Config(PinotConfiguration pinotConfig) {
if (_iamRoleBasedAccess) {
Preconditions.checkNotNull(_roleArn, "Must provide 'roleArn' if iamRoleBasedAccess is enabled");
}
+ PinotConfiguration httpConfig = pinotConfig.subset(HTTP_CLIENT_CONFIG_PREFIX);
+ _httpClientBuilder = httpConfig.isEmpty() ? null : createHttpClientBuilder(httpConfig);
+ }
+
+ private static ApacheHttpClient.Builder createHttpClientBuilder(PinotConfiguration config) {
+ ApacheHttpClient.Builder httpClientBuilder = ApacheHttpClient.builder();
+ String value = config.getProperty(HTTP_CLIENT_CONFIG_MAX_CONNECTIONS);
+ if (value != null) {
+ int pv = Integer.parseInt(value);
+ LOGGER.debug("Set maxConnections to {} for http client builder", pv);
+ httpClientBuilder.maxConnections(pv);
+ }
+ value = config.getProperty(HTTP_CLIENT_CONFIG_SOCKET_TIMEOUT);
+ if (value != null) {
+ Duration pv = parseDuration(value);
+ httpClientBuilder.socketTimeout(pv);
+ LOGGER.debug("Set socketTimeout to {}sec for http client builder", pv.toSeconds());
+ }
+ value = config.getProperty(HTTP_CLIENT_CONFIG_CONNECTION_TIMEOUT);
+ if (value != null) {
+ Duration pv = parseDuration(value);
+ httpClientBuilder.connectionTimeout(pv);
+ LOGGER.debug("Set connectionTimeout to {}sec for http client builder", pv.toSeconds());
+ }
+ value = config.getProperty(HTTP_CLIENT_CONFIG_CONNECTION_TIME_TO_LIVE);
+ if (value != null) {
+ Duration pv = parseDuration(value);
+ httpClientBuilder.connectionTimeToLive(pv);
+ LOGGER.debug("Set connectionTimeToLive to {}sec for http client builder", pv.toSeconds());
+ }
+ value = config.getProperty(HTTP_CLIENT_CONFIG_CONNECTION_ACQUISITION_TIMEOUT);
+ if (value != null) {
+ Duration pv = parseDuration(value);
+ httpClientBuilder.connectionAcquisitionTimeout(pv);
+ LOGGER.debug("Set connectionAcquisitionTimeout to {}sec for http client builder", pv.toSeconds());
+ }
+ return httpClientBuilder;
+ }
+
+ @VisibleForTesting
+ static Duration parseDuration(String durStr) {
+ try {
+ // try format like '1hr20s'
+ return Duration.ofMillis(TimeUtils.convertPeriodToMillis(durStr));
+ } catch (Exception ignore) {
+ }
+ // try format like 'PT1H20S'
+ return Duration.parse(durStr);
Review Comment:
nit: catch exception here too, and put error log message telling user that they need one of the 2 formats
--
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] klsince commented on pull request #10369: allow to customize the httpclient used to create S3PinotFS
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on PR #10369:
URL: https://github.com/apache/pinot/pull/10369#issuecomment-1452400055
> I didn't realize we've already introduced PT notations in DateTimeFunctions. ...
Thanks for the context. I was using PT format as Duration is what the httpClientBuilder takes. I'll make it compatible with the two formats, as it's better to keep format of timeout/period configs consistent
--
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 #10369: allow to customize the httpclient used to create S3PinotFS
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10369:
URL: https://github.com/apache/pinot/pull/10369#issuecomment-1452452812
# [Codecov](https://codecov.io/gh/apache/pinot/pull/10369?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 [#10369](https://codecov.io/gh/apache/pinot/pull/10369?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2db8a7b) into [master](https://codecov.io/gh/apache/pinot/commit/9c7e771d22a0cbd3391593e9976669fff4d4b442?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9c7e771) will **increase** coverage by `0.04%`.
> The diff coverage is `81.08%`.
```diff
@@ Coverage Diff @@
## master #10369 +/- ##
============================================
+ Coverage 13.76% 13.80% +0.04%
- Complexity 231 237 +6
============================================
Files 1979 1979
Lines 107729 107763 +34
Branches 16457 16463 +6
============================================
+ Hits 14831 14881 +50
+ Misses 91717 91695 -22
- Partials 1181 1187 +6
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests2 | `13.80% <81.08%> (+0.04%)` | :arrow_up: |
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/10369?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/plugin/filesystem/S3PinotFS.java](https://codecov.io/gh/apache/pinot/pull/10369?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1maWxlLXN5c3RlbS9waW5vdC1zMy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL2ZpbGVzeXN0ZW0vUzNQaW5vdEZTLmphdmE=) | `44.06% <0.00%> (-0.26%)` | :arrow_down: |
| [...a/org/apache/pinot/plugin/filesystem/S3Config.java](https://codecov.io/gh/apache/pinot/pull/10369?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1maWxlLXN5c3RlbS9waW5vdC1zMy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL2ZpbGVzeXN0ZW0vUzNDb25maWcuamF2YQ==) | `68.83% <85.71%> (+68.83%)` | :arrow_up: |
| [...ntroller/helix/core/minion/TaskMetricsEmitter.java](https://codecov.io/gh/apache/pinot/pull/10369?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9UYXNrTWV0cmljc0VtaXR0ZXIuamF2YQ==) | `86.53% <0.00%> (-1.93%)` | :arrow_down: |
| [...lix/core/minion/PinotHelixTaskResourceManager.java](https://codecov.io/gh/apache/pinot/pull/10369?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdEhlbGl4VGFza1Jlc291cmNlTWFuYWdlci5qYXZh) | `28.63% <0.00%> (-1.12%)` | :arrow_down: |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/pinot/pull/10369?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `62.34% <0.00%> (-0.18%)` | :arrow_down: |
| [...ocal/segment/index/loader/ForwardIndexHandler.java](https://codecov.io/gh/apache/pinot/pull/10369?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9Gb3J3YXJkSW5kZXhIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ces/PinotSegmentUploadDownloadRestletResource.java](https://codecov.io/gh/apache/pinot/pull/10369?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFVwbG9hZERvd25sb2FkUmVzdGxldFJlc291cmNlLmphdmE=) | `39.83% <0.00%> (+0.82%)` | :arrow_up: |
| [...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/pinot/pull/10369?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh) | `80.64% <0.00%> (+5.37%)` | :arrow_up: |
: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