You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "mneedham (via GitHub)" <gi...@apache.org> on 2023/07/27 10:38:12 UTC
[GitHub] [pinot] mneedham opened a new pull request, #11196: Testing out a potential fix
mneedham opened a new pull request, #11196:
URL: https://github.com/apache/pinot/pull/11196
This is a bugfix for an issue that was introduced when upgrading the AWS S3 library from version 2.14.28:
```
- <aws.sdk.version>2.14.28</aws.sdk.version>
+ <aws.sdk.version>2.20.83</aws.sdk.version>
```
This commit:
```
commit 0420414fc2a5408dc7aa32fdb9d359763fc3cb34
Author: Xiaobing <61...@users.noreply.github.com>
Date: Tue Jun 13 10:08:00 2023 -0700
bump awssdk version for a bugfix on http conn leakage (#10898)
* bump awssdk version for a bugfix on http conn leakage
* use latest s3mock and fix S3PinotFSTest
```
This causes a problem when trying to use MinIO, which is S3 API compatible. The default of how URIs are resolved seems to have got messed up for MInIO, resulting in the following exception:
```
Caused by: java.net.UnknownHostException: pinot-controller.minio
at java.net.InetAddress$CachedAddresses.get(InetAddress.java:797) ~[?:?]
at java.net.InetAddress.getAllByName0(InetAddress.java:1533) ~[?:?]
at java.net.InetAddress.getAllByName(InetAddress.java:1386) ~[?:?]
at java.net.InetAddress.getAllByName(InetAddress.java:1307) ~[?:?]
at org.apache.http.impl.conn.SystemDefaultDnsResolver.resolve(SystemDefaultDnsResolver.java:45) ~[pinot-s3-0.13.0-SNAPSHOT-shaded.jar:0.13.0-SNAPSHOT-fe2b013a657e1ad6ac508a9a37933961bc4c408b]
at org.apache.http.impl.conn.DefaultHttpClientConnectionOperator.connect(DefaultHttpClientConnectionOperator.java:112) ~[pinot-s3-0.13.0-SNAPSHOT-shaded.jar:0.13.0-SNAPSHOT-fe2b013a657e1ad6ac508a9a37933961bc4c408b]
at org.apache.http.impl.conn.PoolingHttpClientConnectionManager.connect(PoolingHttpClientConnectionManager.java:376) ~[pinot-s3-0.13.0-SNAPSHOT-shaded.jar:0.13.0-SNAPSHOT-fe2b013a657e1ad6ac508a9a37933961bc4c408b]
at software.amazon.awssdk.http.apache.internal.conn.ClientConnectionManagerFactory$DelegatingHttpClientConnectionManager.connect(ClientConnectionManagerFactory.java:86) ~[pinot-s3-0.13.0-SNAPSHOT-shaded.jar:0.13.0-SNAPSHOT-fe2b013a657e1ad6ac508a9a37933961bc4c408b]
```
This [StackOverflow thread ](https://stackoverflow.com/questions/72205086/amazonss3client-throws-unknownhostexception-if-attempting-to-connect-to-a-local) describes the issue and a way to work around it.
I'm not sure whether setting this flag would cause an issue for other S3 API implementations.
--
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 #11196: Fix for issue when using MinIO as deep store
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11196:
URL: https://github.com/apache/pinot/pull/11196#issuecomment-1653408010
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11196?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11196](https://app.codecov.io/gh/apache/pinot/pull/11196?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (783c8e3) into [master](https://app.codecov.io/gh/apache/pinot/commit/61dcea6b71a99746805bc7f322daed67f9bce265?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (61dcea6) will **increase** coverage by `0.00%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11196 +/- ##
=========================================
Coverage 0.11% 0.11%
=========================================
Files 2223 2168 -55
Lines 119274 116722 -2552
Branches 18055 17743 -312
=========================================
Hits 137 137
+ Misses 119117 116565 -2552
Partials 20 20
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1temurin11 | `?` | |
| integration1temurin17 | `?` | |
| integration1temurin20 | `?` | |
| integration2temurin11 | `?` | |
| integration2temurin17 | `?` | |
| integration2temurin20 | `?` | |
| unittests1temurin11 | `?` | |
| unittests1temurin17 | `?` | |
| unittests1temurin20 | `?` | |
| unittests2temurin11 | `?` | |
| unittests2temurin17 | `?` | |
| unittests2temurin20 | `0.11% <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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files Changed](https://app.codecov.io/gh/apache/pinot/pull/11196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../org/apache/pinot/plugin/filesystem/S3PinotFS.java](https://app.codecov.io/gh/apache/pinot/pull/11196?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcGx1Z2lucy9waW5vdC1maWxlLXN5c3RlbS9waW5vdC1zMy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL2ZpbGVzeXN0ZW0vUzNQaW5vdEZTLmphdmE=) | `0.00% <0.00%> (ø)` | |
... and [57 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11196/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
: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=apache)
--
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 a diff in pull request #11196: Fix for issue when using MinIO as deep store
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11196:
URL: https://github.com/apache/pinot/pull/11196#discussion_r1299287253
##########
pinot-plugins/pinot-file-system/pinot-s3/src/main/java/org/apache/pinot/plugin/filesystem/S3PinotFS.java:
##########
@@ -138,8 +138,10 @@ public void init(PinotConfiguration config) {
.asyncCredentialUpdateEnabled(s3Config.isAsyncSessionUpdateEnabled()).build();
}
- S3ClientBuilder s3ClientBuilder =
- S3Client.builder().region(Region.of(s3Config.getRegion())).credentialsProvider(awsCredentialsProvider);
+ S3ClientBuilder s3ClientBuilder = S3Client.builder()
+ .forcePathStyle(true)
+ .region(Region.of(s3Config.getRegion()))
+ .credentialsProvider(awsCredentialsProvider);
Review Comment:
```suggestion
S3ClientBuilder s3ClientBuilder = S3Client.builder().forcePathStyle(true).region(Region.of(s3Config.getRegion()))
.credentialsProvider(awsCredentialsProvider);
```
--
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 #11196: Fix for issue when using MinIO as deep store
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #11196:
URL: https://github.com/apache/pinot/pull/11196#issuecomment-1656147606
@klsince Please also take a look
--
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 #11196: Fix for issue when using MinIO as deep store
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #11196:
URL: https://github.com/apache/pinot/pull/11196
--
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] pjpringle commented on pull request #11196: Fix for issue when using MinIO as deep store
Posted by "pjpringle (via GitHub)" <gi...@apache.org>.
pjpringle commented on PR #11196:
URL: https://github.com/apache/pinot/pull/11196#issuecomment-1681554931
Looks good. Although AWS will be deprecating this path style access sometime in the future.
It may be worth introducing a config property to control pathStyleAccess. e.g. `pinot.server.storage.factory.s3.pathStyleAccess=true`
--
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