You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "lucifer4j (via GitHub)" <gi...@apache.org> on 2023/06/30 06:50:35 UTC
[GitHub] [pinot] lucifer4j opened a new pull request, #11014: Fix double-checked locking in ConnectionFactory
lucifer4j opened a new pull request, #11014:
URL: https://github.com/apache/pinot/pull/11014
`bugfix` `pinot-client`
The `_defaultTransport` in `ConnectionFactory` is created using double-checked locking.
https://github.com/apache/pinot/blob/03b9d4a708e6d09838e902857ebe1f255ced4ba1/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ConnectionFactory.java#L211-L222
However, the current implementation is buggy as described in https://rules.sonarsource.com/java/RSPEC-2168/. To fix it, make the field `volatile`.
--
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 #11014: Fix double-checked locking in ConnectionFactory
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #11014:
URL: https://github.com/apache/pinot/pull/11014#issuecomment-1626248999
@lucifer4j : just curious, did you see issues related to this or is this a follow-up from a static code-analysis report?
--
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] lucifer4j commented on pull request #11014: Fix double-checked locking in ConnectionFactory
Posted by "lucifer4j (via GitHub)" <gi...@apache.org>.
lucifer4j commented on PR #11014:
URL: https://github.com/apache/pinot/pull/11014#issuecomment-1626898927
@Jackie-Jiang Hi! Sorry for the delay, I can add the link but I see the PR is already merged now.
@ankitsultana I was working on this file as part of a different PR when I noticed this issue being pointed out by static analysis. AFAIU, this is a benign race condition as multiple PinotClientTransport's can co-exist without bugs.
--
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 #11014: Fix double-checked locking in ConnectionFactory
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11014:
URL: https://github.com/apache/pinot/pull/11014#issuecomment-1614246056
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11014?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11014](https://app.codecov.io/gh/apache/pinot/pull/11014?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (60b5381) into [master](https://app.codecov.io/gh/apache/pinot/commit/379d7610ac58724d1dcfe5968ddad8bb37a988d3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (379d761) will **increase** coverage by `0.00%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #11014 +/- ##
=========================================
Coverage 0.11% 0.11%
=========================================
Files 2192 2138 -54
Lines 118016 115513 -2503
Branches 17869 17567 -302
=========================================
Hits 137 137
+ Misses 117859 115356 -2503
Partials 20 20
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1temurin11 | `?` | |
| integration1temurin17 | `?` | |
| integration1temurin20 | `?` | |
| integration2temurin11 | `?` | |
| integration2temurin17 | `?` | |
| integration2temurin20 | `?` | |
| unittests1temurin11 | `?` | |
| unittests1temurin17 | `?` | |
| unittests1temurin20 | `?` | |
| unittests2temurin11 | `0.11% <ø> (ø)` | |
| unittests2temurin17 | `0.11% <ø> (ø)` | |
| unittests2temurin20 | `?` | |
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.
| [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/11014?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ava/org/apache/pinot/client/ConnectionFactory.java](https://app.codecov.io/gh/apache/pinot/pull/11014?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb25GYWN0b3J5LmphdmE=) | `0.00% <ø> (ø)` | |
... and [54 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11014/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] xiangfu0 merged pull request #11014: Fix double-checked locking in ConnectionFactory
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #11014:
URL: https://github.com/apache/pinot/pull/11014
--
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