allthingssecurity opened a new pull request, #29669:
URL: https://github.com/apache/camel/pull/29669

   # Description
   
   [CAMEL-26868](https://issues.apache.org/jira/browse/CAMEL-26868)
   
   `JdbcCachedMessageIdRepository.remove` (and `clear`) removes the key from 
its cache before and after the delete. In a transaction of the caller on the 
same DataSource (or JTA) the delete joins that transaction and is only 
committed later: a direct `remove` or `clear` in a transaction, or the 
idempotent consumer removing the key of a failure that a dead letter channel 
handled, inside an enclosing transaction that commits (such as a 
`ProducerTemplate` call in a transactional service). Until then another thread 
still sees the row: its `add` of the key finds it, returns false (a duplicate) 
and caches the key. When the caller's transaction commits the row is gone, but 
the key stays in the cache, so every later `add` of that key is rejected as a 
duplicate without a row, until the cache is reloaded. A failure that rolls the 
transaction back undoes the delete, so a plain failed exchange of a transacted 
route is not affected. Found in the review of #29533 (CAMEL-26770), which 
caches a key 
 only after the commit of its row.
   
   Change: `remove` (when it deleted a row) and `clear` also remove the key (or 
every key) from the cache when the caller's transaction commits, with a 
transaction synchronization as `add` uses since #29533. Without a transaction 
with synchronization the delete has committed when `remove` returns, as before, 
and nothing is registered. A remove and then an add of the key in one 
transaction still leave the key cached (the synchronizations run in the order 
they were registered), and a remove in a transaction that rolls back keeps the 
row (the key is only out of the cache, which costs a cache miss).
   
   This narrows the window, it does not close it: an `add` on another thread 
whose SELECT saw the row before the delete committed, and that caches the key 
only after the eviction ran (without a transaction of its own, when it caches 
right after its SELECT; or in a transaction that commits after the remove's), 
still leaves the key cached. The comment of #29533 next to the second 
`cache.remove` already says the same of the remove without a caller 
transaction. Closing it needs the add to know that an eviction ran meanwhile 
(for example a counter of evictions read before the add); I will offer that as 
a follow-up. With a caller transaction of another transaction manager (another 
DataSource, JMS) the delete commits in a transaction of its own, and the 
registered eviction only costs a cache miss.
   
   Main only: it builds on #29533 (CAMEL-26770, main only); the older lines 
have the same stale key, without the caching after commit.
   
   Tests, in `JdbcCachedMessageIdRepositoryTransactionTest` (H2; the caller's 
transaction runs on another thread and waits on a latch, so the add runs while 
it is open):
   - 
`removeInACallerTransactionEvictsTheKeyThatAnotherThreadCachedBeforeTheCommit` 
and 
`clearInACallerTransactionEvictsTheKeyThatAnotherThreadCachedBeforeTheCommit`: 
the add on the other thread is a duplicate (the row is still visible); after 
the commit the key is not in the cache and can be added again.
   - `removeInACallerTransactionThatRollsBackKeepsTheRow` and 
`removeAndAddInOneCallerTransactionIsCached`: controls (they pass before the 
change too).
   
   Without the change the first two fail (`contains` still answers true from 
the cache). With it the camel-sql tests pass (368 run, 0 failures, without 
`SqlFunctionDataSourceTest`, whose embedded MariaDB cannot start here), and 
`JdbcCachedMessageIdRepositoryTransactionTest` passes 180/180 over 20 runs in 
one JVM.
   
   No upgrade guide entry and no documentation change: a fix of a race, no 
option or default changes.
   
   # 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-26868](https://issues.apache.org/jira/browse/CAMEL-26868).
   
   # 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-sql with the formatter and import-sort 
plugins and ran the camel-sql 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]

Reply via email to