allthingssecurity opened a new pull request, #29434: URL: https://github.com/apache/camel/pull/29434
# Description [CAMEL-26740](https://issues.apache.org/jira/browse/CAMEL-26740) Two problems with the expiry handling of `KafkaKeyValueRepository` (camel-kafka, since 4.23): - `keys()` and `size()` (also the JMX attribute `Size`) first call `evictExpired()`, which removed expired entries with `Iterator.remove()` on `cache.entrySet().iterator()`. The cache comes from `LRUCacheFactory`, by default a `SimpleLRUCache`, whose `entrySet()` is an unmodifiable copy (`Collectors.toUnmodifiableSet()`). So once one entry with a time to live had expired, every `keys()` and `size()` failed with `UnsupportedOperationException: remove`, and so did their callers, such as the `keys` and `size` operations of the state-store component. The class Javadoc says entries replayed from the topic on startup "will appear as expired immediately", so after a restart this could happen at once. - `putIfAbsent` read the entry, removed it if expired, then called `cache.putIfAbsent`. When another writer stored an entry for the key in between (another thread, or the topic poller applying a record whose expiry time had already passed, which happens when it lags behind by more than the time to live) and that entry had expired, it returned `null`, which the interface, the method's Javadoc and the state-store docs define as "stored", without storing or sending its value. The caller then acted as the owner of a key that no other instance, and no restart, could see. Change: - `evictExpired()` iterates the entry set and removes through the cache, `cache.remove(key, entry)`, so a key that was put again meanwhile is kept (the old iterator removed the key whatever it held). - `putIfAbsent` first calls `cache.putIfAbsent`; when it finds an expired entry it replaces it atomically (`cache.replace(key, expired, newEntry)`), and when the key changed meanwhile it looks again. It returns `null` only when its own entry is in the cache, then sends it as before (the rollback on a failed send is unchanged). This is the approach of `MemoryKeyValueRepository.putIfAbsent`, plus the retry: Memory's last fallback can also return `null` without storing (and its `evictExpired` removes the key whatever it holds); that is left for a separate issue. Tests: a new `KafkaKeyValueRepositoryTest` (no broker: the changelog sends to a `MockProducer`, topic records are applied through the repository's poller callback, the cache is a `SimpleLRUCache` as `LRUCacheFactory` creates by default): - `keys()` and `size()` with one live and one expired entry return the live one, and the expired one is evicted; - `putIfAbsent` over an expired entry stores and sends the value; - `putIfAbsent` when the poller applies an expired record of the key just before it stores its own (a map whose `putIfAbsent` applies the record first) returns `null`, the value is readable and was sent; - `putIfAbsent` that finds an expired entry, while the poller applies a live record of the key just before it replaces that entry, returns the live value and sends nothing (the retry of the loop); - `putIfAbsent` returns the value of a live entry and sends nothing. On main 4 of the 6 tests fail (`UnsupportedOperationException: remove` twice, `expected: <mine> but was: <null>`, and `expected: <peer> but was: <null>` for the retry test, which covers the new loop: main never calls `replace`); with the fix 6/6, and 120/120 over 20 runs. camel-kafka unit tests (`-DskipITs`, offline, with the core modules in the reactor): 312 run, 0 failures, 1 skipped; the formatter and import-sort plugins changed nothing. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it): [CAMEL-26740](https://issues.apache.org/jira/browse/CAMEL-26740). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built the core modules and camel-kafka with the formatter and import-sort plugins and ran the camel-kafka unit tests; no generated file changes. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
