CAMEL-23840: camel-core: pollEnrich with cacheSize(-1) does not disable consumer cache (dynamic endpoints)#24283
CAMEL-23840: camel-core: pollEnrich with cacheSize(-1) does not disable consumer cache (dynamic endpoints)#24283msnijder30 wants to merge 3 commits into
Conversation
…ng consumer cache The existing test only verified that the PollEnricher configured getCacheSize() returns -1 and that dynamic endpoints are not leaked into the endpoint registry. It did not verify that polling consumers are actually discarded after use — the documented behavior of cacheSize(-1). Add a test-only component whose PollingConsumer instances count doStop() calls, proving that with cacheSize(-1) consumers remain in the DefaultConsumerCache instead of being stopped. The test currently fails, demonstrating the bug. Co-authored-by: OpenCode (DeepSeek V4 Pro) <contact@anoma.ly>
When cacheSize is set to -1 the consumer cache should be disabled and polling consumers should be created fresh for each call and discarded immediately after use. However DefaultConsumerCache normalizes any cacheSize <= 0 to the CamelContext maximum (default 1000), causing consumers to be retained in a bounded cache. Fix by: - Skipping creation of DefaultConsumerCache in doBuild() when cacheSize < 0 - Creating/stopping consumers directly in process() when consumerCache is null - Adding null guard in getEndpointUtilizationStatistics() This mirrors how DefaultProducerCache with cacheSize=-1 (and EmptyProducerCache) already works for the producer side. Co-authored-by: OpenCode (DeepSeek V4 Pro) <contact@anoma.ly>
msnijder30
changed the title
msnijder30
changed the title
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
Build reactor — dependencies compiled but only changed modules were tested (3 modules)
|
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for the fix and the clear root-cause analysis — the bug is real and the test is well done.
However, the approach of setting consumerCache = null and scattering if (consumerCache != null) checks diverges from the established pattern on the producer side. SendDynamicProcessor, RecipientList, and RoutingSlip all use EmptyProducerCache (extends DefaultProducerCache) when cacheSize < 0 — it overrides acquireProducer() to always create new, and releaseProducer() to always stop/shutdown. No null checks needed in the calling code.
Please create an EmptyConsumerCache (extends DefaultConsumerCache) in core/camel-support/src/main/java/org/apache/camel/support/cache/ that mirrors EmptyProducerCache, and use it in PollEnricher.doBuild() so that consumerCache is never null. This keeps the codebase consistent and avoids the null checks.
The test additions are great — please keep those.
This review does not replace specialized AI review tools or static analysis.
This review was generated by an AI agent (Claude Code on behalf of Claus Ibsen) and may contain inaccuracies. Please verify all suggestions before applying.
…e pattern The previous fix for pollEnrich cacheSize=-1 skipped creation of the consumer cache and scattered `if (consumerCache != null)` guards through PollEnricher. This diverges from the established producer-side pattern, where SendDynamicProcessor / RecipientList / RoutingSlip use EmptyProducerCache (extends DefaultProducerCache) so the cache reference is never null and the acquire/release contract stays uniform. Address the peer-review feedback by mirroring the producer side: - Add EmptyConsumerCache (extends DefaultConsumerCache) that overrides acquirePollingConsumer() to always create+start a fresh consumer and releasePollingConsumer() to stopAndShutdown it, and returns 0 from getCapacity(). Equivalent to EmptyProducerCache. - Update DefaultConsumerCache to conditionally create its service pool only when cacheSize >= 0 (mirroring DefaultProducerCache), and null- guard size()/purge()/cleanUp() so the EmptyConsumerCache base instance does not touch the pool. - Rewrite PollEnricher.doBuild() to pick EmptyConsumerCache when cacheSize < 0 and DefaultConsumerCache otherwise, matching SendDynamicProcessor.doBuild(). Remove the now-redundant null guards in getEndpointUtilizationStatistics() and the acquire/release sites; they collapse back to unconditional consumerCache.* calls. - Add EmptyConsumerCacheTest mirroring EmptyProducerCacheTest, asserting size() stays 0 across 1000+ acquire/release cycles. Co-authored-by: OpenCode (GLM 5.2) <contact@anoma.ly>
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Thanks for the update — this now correctly mirrors the EmptyProducerCache / DefaultProducerCache pattern. The DefaultConsumerCache changes match DefaultProducerCache's null-pool handling, EmptyConsumerCache mirrors EmptyProducerCache, and PollEnricher.doBuild() follows SendDynamicProcessor.doBuild(). Clean, consistent, and well tested.
This review was generated by an AI agent (Claude Code on behalf of Claus Ibsen) and may contain inaccuracies. Please verify all suggestions before applying.
3 participants
Description
I found out about this issue with pollEnrich because we had a memory issue with the SFTP component because we did not set the cache size so it defaulted to 1000. So I read the documentation and set it to cachesize to -1, but the memory still kept going up.
Our code was using dynamic endpoints that triggered this.
pollEnrich().cacheSize(-1) is documented to disable the consumer cache entirely - polling consumers should be created fresh for each call and discarded immediately after use. However, this does not appear to work.
Root cause: PollEnricher.doBuild() unconditionally creates a DefaultConsumerCache regardless of cacheSize. DefaultConsumerCache then normalizes any cacheSize <= 0 to the CamelContext maximum cache pool size (default 1000):
So instead of discarding consumers, up to 1000 polling consumers are retained in a cache.
For resource-backed components (e.g., SFTP), each retained consumer represents an open connection that is never cleaned up until the route stops.
Reproducer
You can find my reproducer here: https://github.com/msnijder30/reproduce-camel-pollenrich-cache-issue
Also attached as zip to the JIRA ticket.
The fix
This mirrors how DefaultProducerCache (via EmptyProducerCache) already works on the producer side for cacheSize(-1).
Didn't create a DefaultConsumerCache class because it's only used in this class, unlike DefaultProducerCache which is used in multiple places.
Let me know what you think 👍 it was interesting to run into this bug
Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.