You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "orionlibs (via GitHub)" <gi...@apache.org> on 2023/07/15 16:48:33 UTC
[GitHub] [commons-pool] orionlibs opened a new pull request, #231: POOL-411: implemented maxConcurrentConnections stat
orionlibs opened a new pull request, #231:
URL: https://github.com/apache/commons-pool/pull/231
(no comment)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-pool] aherbert commented on pull request #231: POOL-410: implemented maxConcurrentConnections stat
Posted by "aherbert (via GitHub)" <gi...@apache.org>.
aherbert commented on PR #231:
URL: https://github.com/apache/commons-pool/pull/231#issuecomment-1636846334
This is tracking the _number_ of concurrent connections, not the _max observed_. Please check the requirements in POOL-410.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-pool] psteitz commented on pull request #231: POOL-410: implemented maxConcurrentConnections stat
Posted by "psteitz (via GitHub)" <gi...@apache.org>.
psteitz commented on PR #231:
URL: https://github.com/apache/commons-pool/pull/231#issuecomment-1636926034
Remembering now that we provide stats caching for timing metrics, I think it is reasonable to ask for that for active and idle counts. I take back my grumpiness above. I would be happy to review a PR to add activeCounts and idleCounts to go along with activeTimes and the other latency StatsStore instances now in BaseGenericObjectPool. I would approach that by creating activeCounts and idleCounts as StatsStore instances in BGOP and updating them in updateStatsOnReturn and updateStatsOnBorrow, using current active and idle counts from the pool, Then include these stats in reports. And tests.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-pool] codecov-commenter commented on pull request #231: POOL-410: implemented maxConcurrentConnections stat
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #231:
URL: https://github.com/apache/commons-pool/pull/231#issuecomment-1636835350
## [Codecov](https://app.codecov.io/gh/apache/commons-pool/pull/231?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#231](https://app.codecov.io/gh/apache/commons-pool/pull/231?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (44616a9) into [master](https://app.codecov.io/gh/apache/commons-pool/commit/f9c6b3e68da4f178b971a4bae2110b06e87077c7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (f9c6b3e) will **decrease** coverage by `0.13%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #231 +/- ##
============================================
- Coverage 83.50% 83.37% -0.13%
+ Complexity 787 785 -2
============================================
Files 42 42
Lines 3067 3074 +7
Branches 308 308
============================================
+ Hits 2561 2563 +2
- Misses 406 409 +3
- Partials 100 102 +2
```
| [Impacted Files](https://app.codecov.io/gh/apache/commons-pool/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ache/commons/pool2/impl/BaseGenericObjectPool.java](https://app.codecov.io/gh/apache/commons-pool/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvcG9vbDIvaW1wbC9CYXNlR2VuZXJpY09iamVjdFBvb2wuamF2YQ==) | `88.75% <100.00%> (+0.13%)` | :arrow_up: |
| [...g/apache/commons/pool2/impl/GenericObjectPool.java](https://app.codecov.io/gh/apache/commons-pool/pull/231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvcG9vbDIvaW1wbC9HZW5lcmljT2JqZWN0UG9vbC5qYXZh) | `86.29% <100.00%> (+0.06%)` | :arrow_up: |
... and [2 files with indirect coverage changes](https://app.codecov.io/gh/apache/commons-pool/pull/231/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: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-pool] garydgregory commented on pull request #231: POOL-410: implemented maxConcurrentConnections stat
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #231:
URL: https://github.com/apache/commons-pool/pull/231#issuecomment-1636913061
I'm ok to close this.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-pool] orionlibs closed pull request #231: POOL-410: implemented maxConcurrentConnections stat
Posted by "orionlibs (via GitHub)" <gi...@apache.org>.
orionlibs closed pull request #231: POOL-410: implemented maxConcurrentConnections stat
URL: https://github.com/apache/commons-pool/pull/231
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-pool] psteitz commented on pull request #231: POOL-410: implemented maxConcurrentConnections stat
Posted by "psteitz (via GitHub)" <gi...@apache.org>.
psteitz commented on PR #231:
URL: https://github.com/apache/commons-pool/pull/231#issuecomment-1636895334
Tbe added property appears to just duplicate numActive, which is already there.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-pool] orionlibs commented on pull request #231: POOL-410: implemented maxConcurrentConnections stat
Posted by "orionlibs (via GitHub)" <gi...@apache.org>.
orionlibs commented on PR #231:
URL: https://github.com/apache/commons-pool/pull/231#issuecomment-1636852293
> This is tracking the _number_ of concurrent connections, not the _max observed_. Please check the requirements in POOL-410.
You are right. The ticket is very difficult to understand. I added support for maxConcurrentConnectionsObserved. I hope that is correct now
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-pool] psteitz commented on pull request #231: POOL-410: implemented maxConcurrentConnections stat
Posted by "psteitz (via GitHub)" <gi...@apache.org>.
psteitz commented on PR #231:
URL: https://github.com/apache/commons-pool/pull/231#issuecomment-1636898718
I am inclined to reject this as even with the fix to use numActive, I don't see the value to justify the added compute in updateBorrowStats (which is a hot method). If clients want to maintain stats over specific timeframes, they can do that already and they should set the timeframes.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org