allthingssecurity opened a new pull request, #29437: URL: https://github.com/apache/camel/pull/29437
# Description [CAMEL-26743](https://issues.apache.org/jira/browse/CAMEL-26743) `JdbcOrphanLockAwareIdempotentRepository` guards its set of held keys with a `StampedLock`, and held the lock while SQL ran: - `keepAlive()` took the read lock, and then started its transaction, which borrows a connection from the pool. - `add()` and `remove()` borrow a connection first (the transaction of `AbstractJdbcMessageIdRepository`), and then took the write lock in `insert` / `delete`, around the COUNT, INSERT, UPDATE or DELETE. With more threads using the repository than the pool has connections, the two orders form a cycle: the keep-alive holds the read lock and waits for a connection, while every connection is held by an `add` or `remove` waiting for the write lock. With a pool that waits as long as it takes (the defaults of plain Agroal and DBCP2) the node deadlocks: no `add` or `remove` completes again, and the keep-alive never runs. With a pool timeout (HikariCP: 30 s) the keep-alive round fails after the timeout, only logged at ERROR, and the `add`/`remove` calls stall until then; under steady load the next round can be caught the same way. With an infinite wait no lease is refreshed again; with a timeout, when rounds keep failing for `lockMaxAgeMillis`, the leases expire. Then another node takes over keys this node is still processing (the orphan takeover of CAMEL-23294 then does what it should, for a lease that only looks orphaned). The same lock made the keep-alive wait for any slow statement of an `add` or `remove`, such as an INSERT waiting for a row lock that another node holds (a transacted route there with an uncommitted insert of the same key). PostgreSQL, Oracle and SQL Server have no lock timeout by default. The lock protected the in-memory set. It did not make COUNT and INSERT atomic, since the transaction of `add` or `remove` commits after the lock is released; its only other effect was that a keep-alive round did not overlap an `add` or `remove` of the same node. Change: - The held keys are in a concurrent set (`ConcurrentHashMap.newKeySet()`), and no lock is held across SQL. `insert`, `delete` and `delete()` run their SQL and then update the set, as before. - `keepAlive()` takes its timestamp and the keys inside the transaction, once it has its connection (with the default `DataSourceTransactionManager`), so a round that waited for a connection does not write a timestamp from before the wait. What changes in the timing: a keep-alive round can now overlap a `remove` of the same node. If the round read the key before the `remove`, and another node inserts the key again before the round's UPDATE, that UPDATE refreshes the other node's new lease once (with the current time). This only delays a later takeover of that key by at most one keep-alive interval. What is left as it was: - the timestamp of `insert` is still taken before its statements, so an INSERT that waits on a row lock of another node still writes the time from before that wait (using the database clock is a larger change); - the key is added to the set before the transaction of `add` commits, so a failed commit leaves it in the set. These can be follow-ups. Tests (H2 embedded database, `JdbcOrphanLockAwareIdempotentRepositoryKeepAliveTest`, no Docker). A small test pool over the embedded database waits for a free connection as long as it takes, and can hold one thread after it borrowed a connection or before a statement: - one connection: `add` (and `remove`) borrows the connection and is held; the keep-alive then waits for the connection; `add` goes on. Both must finish, and the keep-alive must refresh a held key. - two connections: `add` is held before its INSERT, as a statement waiting for a row lock would be. The keep-alive must finish and refresh a held key meanwhile. On main all three fail: in the first two the thread dump shows the cycle (`add` or `remove` parked in `StampedLock.writeLock`, the keep-alive waiting for the connection), in the third the keep-alive is parked in `readLock` behind the INSERT; with the fix 3/3, and 220/220 over 20 runs together with the other orphan-lock tests (`...Test`, `...ConcurrencyTest`, `...RestartTest`). camel-sql module tests on current main: 348 run, 0 failures, 3 skipped (with the core modules in the reactor); `SqlFunctionDataSourceTest`, which installs an embedded MariaDB, was not run locally (CI runs it). The fix builds on CAMEL-23294 and CAMEL-26705, so a backport to an older branch is not a plain cherry-pick. # 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-26743](https://issues.apache.org/jira/browse/CAMEL-26743). # 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]
