You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "andreachild (via GitHub)" <gi...@apache.org> on 2025/04/01 00:24:12 UTC
[PR] Draft: fix for connection pool bug where the waitLock can be signaled … [tinkerpop]
andreachild opened a new pull request, #3079:
URL: https://github.com/apache/tinkerpop/pull/3079
…before a thread attempting to retrieve a connection calls await, causing 16 second wait for obtaining a connection.
<!--
Thanks for contributing! Reminders:
+ TARGET the earliest branch where you want the change
3.7-dev -> 3.7.4 (non-breaking only)
3.8-dev -> 3.8.0
master -> 4.0.0
+ Committers will MERGE the PR forward to newer versions
+ ADD entry to the CHANGELOG.asciidoc for the targeted version
Do not reference a JIRA number there
+ ADD JIRA number to title and link in description
+ PRs requires 3 +1s from committers OR
1 +1 and 7 day wait to merge.
+ MORE details: https://s.apache.org/rtnal
-->
--
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@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix connection pool bug which can cause delays in obtaining a connection [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3079:
URL: https://github.com/apache/tinkerpop/pull/3079#discussion_r2031664247
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java:
##########
@@ -526,20 +537,20 @@ private Connection getAvailableConnection() {
private void awaitAvailableConnection(long timeout, TimeUnit unit) throws InterruptedException {
Review Comment:
Nit: I'm a bit worried of the potential for future mistakes now that the caller is responsible for obtaining `waitLock` before calling this method. A javadoc comment here, and on `waitForConnection()` might be helpful.
--
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@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix connection pool bug which can cause delays in obtaining a connection [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer merged PR #3079:
URL: https://github.com/apache/tinkerpop/pull/3079
--
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@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix connection pool bug which can cause delays in obtaining a connection [tinkerpop]
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on PR #3079:
URL: https://github.com/apache/tinkerpop/pull/3079#issuecomment-2790478008
VOTE +1
--
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@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Draft: fix for connection pool bug where the waitLock can be signaled … [tinkerpop]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3079:
URL: https://github.com/apache/tinkerpop/pull/3079#issuecomment-2767739705
## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/3079?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `66.66667%` with `9 lines` in your changes missing coverage. Please review.
> Project coverage is 78.10%. Comparing base [(`cfd6889`)](https://app.codecov.io/gh/apache/tinkerpop/commit/cfd68897ea111e3ae7100dbc2f0f7a364fd0ffa5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`07fa751`)](https://app.codecov.io/gh/apache/tinkerpop/commit/07fa751f6465ffdae91af9109cacfe5d5af5c644?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 195 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/tinkerpop/pull/3079?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...pache/tinkerpop/gremlin/driver/ConnectionPool.java](https://app.codecov.io/gh/apache/tinkerpop/pull/3079?src=pr&el=tree&filepath=gremlin-driver%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Ftinkerpop%2Fgremlin%2Fdriver%2FConnectionPool.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JlbWxpbi1kcml2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL2RyaXZlci9Db25uZWN0aW9uUG9vbC5qYXZh) | 66.66% | [8 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/tinkerpop/pull/3079?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3079 +/- ##
============================================
+ Coverage 77.87% 78.10% +0.22%
- Complexity 13578 13962 +384
============================================
Files 1015 1019 +4
Lines 59308 60025 +717
Branches 6835 6969 +134
============================================
+ Hits 46184 46880 +696
- Misses 10817 10819 +2
- Partials 2307 2326 +19
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/3079?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix connection pool bug which can cause delays in obtaining a connection [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #3079:
URL: https://github.com/apache/tinkerpop/pull/3079#issuecomment-2784033424
VOTE +1
--
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@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix connection pool bug which can cause delays in obtaining a connection [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #3079:
URL: https://github.com/apache/tinkerpop/pull/3079#issuecomment-2787909650
VOTE +1
--
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@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix connection pool bug which can cause delays in obtaining a connection [tinkerpop]
Posted by "andreachild (via GitHub)" <gi...@apache.org>.
andreachild commented on code in PR #3079:
URL: https://github.com/apache/tinkerpop/pull/3079#discussion_r2036207546
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ConnectionPool.java:
##########
@@ -526,20 +537,20 @@ private Connection getAvailableConnection() {
private void awaitAvailableConnection(long timeout, TimeUnit unit) throws InterruptedException {
Review Comment:
Technically calling this method on its own does not mandate acquiring the lock. The issue was the combination of calling `getAvailableConnection` and then subsequently `waitForConnection` without the lock. I have added a comment inside of the `borrowConnection` to clarify.
--
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@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org