Skip to content

Made statistics collection optional in BaseGenericObjectPool#429

Merged
garydgregory merged 9 commits intoapache:masterfrom
Pratyay:master
Dec 1, 2025
Merged

Made statistics collection optional in BaseGenericObjectPool#429
garydgregory merged 9 commits intoapache:masterfrom
Pratyay:master

Conversation

@Pratyay
Copy link
Copy Markdown
Contributor

@Pratyay Pratyay commented Sep 5, 2025

Problem Statement
While analyzing pool performance under high load, I identified that detailed statistics collection (mean active time, mean idle time, mean borrow wait time) can become a bottleneck in high-throughput scenarios. These statistics, while valuable for monitoring, involve timing calculations and circular buffer operations that add overhead to every borrow/return operation. For applications that prioritize throughput over detailed monitoring, there is no way to disable this overhead, and it leads to useful threads blocked on the synchronized block.

Proposed Solution
Implement a new configuration option collectDetailedStatistics that allows users to disable detailed timing statistics collection.

@garydgregory
Copy link
Copy Markdown
Member

Hi @psteitz
Any thoughts on this one?

@garydgregory
Copy link
Copy Markdown
Member

Hello @Pratyay
What application were you analyzing and how?

@Pratyay
Copy link
Copy Markdown
Contributor Author

Pratyay commented Nov 30, 2025

Hello @garydgregory, thanks for the revert
This was while running load on one of our production stateless service, that uses Redis (our L1 cache) via Jedis client.
Overall cluster level, we support around 8M gets/s per datacenter (we have our own datacenters), at pod (for main application container) this is around 30k requests/sec.
Service runs a Dropwizard application container over REST/HTTP with JDK version : jdk8u452-b09 (we are in process of upgrading to JDK17, and eventually to JDK21)
GC configuration: G1GC, Xmx:16GB
The service p95 latency guarantees to our clients are sub 10ms.
We captured detailed JFR analysis and saw contention across threads on the
org.apache.commons.pool2.impl.BaseGenericObjectPool$StatsStore, with average blocked time per thread being ~ 8ms, hitting our latency guarantees.

Screenshot 2025-11-30 at 10 40 56 AM Screenshot 2025-11-30 at 10 42 02 AM

@garydgregory garydgregory changed the title Made statistic collection optional Made statistic collection optional in BaseGenericObjectPool Dec 1, 2025
@garydgregory garydgregory changed the title Made statistic collection optional in BaseGenericObjectPool Made statistics collection optional in BaseGenericObjectPool Dec 1, 2025
@garydgregory garydgregory merged commit abfb125 into apache:master Dec 1, 2025
7 checks passed
@garydgregory
Copy link
Copy Markdown
Member

@Pratyay
Thank you for your contribution. I made changes to this PR and merged it.

If you'd like to see this feature in the 2.x release, line please create a PR against the branch named POOL_2_X.

@Pratyay
Copy link
Copy Markdown
Contributor Author

Pratyay commented Dec 2, 2025

Thanks @garydgregory.
Here is the PR for pool2 package
https://github.com/apache/commons-pool/pull/449/files

@psteitz
Copy link
Copy Markdown
Contributor

psteitz commented Dec 8, 2025

Hi @psteitz Any thoughts on this one?

Sorry to be late to the party. Looks like a reasonable request and impl looks good. Many thanks for the additional tests. I am tempted to try to improve the stats store concurrency tests to do more validation of values, but that will be a little tricky. I guess its OK that JMX will return 0's for the un-tracked stats when turned off. +1 to merge into 2_X as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants