fix pooled connections leaking when a CRUD mutation fails at commit - #624
Merged
Conversation
azahnen
approved these changes
Sep 2, 2026
CRUD writes (POST/PUT/PATCH/DELETE on items) ran through the rxjava3-jdbc transacted chain, which returns the connection to the pool only when its internal reference counter reaches zero. A COMMIT rejected by the database (deferred constraint, constraint trigger, serialization failure at commit) drives the counter below zero and the lease is never released; enough of these starve the pool and every request fails with "Connection is not available". The CRUD entry points of the SQL feature provider now run through the same JDBC session as the transactions module: one session per mutation, committed on success, rolled back on error, and the connection is returned in a finally block. The rxjava3-jdbc write path (getMutationSource, getMutationFlow, the creator/updater/deletion flows and the transaction guard) is removed. The statements are unchanged, PATCH keeps its delete-and-insert semantics, and a POST with several features is now a single transaction. Errors caused by the submitted data keep being reported as a bad request; a lost connection or an exhausted pool is now reported as a server error instead of "Invalid feature data". The session's connection release is hardened as well: resetting autocommit and closing are isolated so a connection the database has terminated is still given back, a failed COMMIT finalises the session, and the connection is released exactly once.
Sessions and statements without a result (creating result-set tables, routes DDL) obtained their connection through the rxjava3-jdbc Database. They now lease it from the pool directly and run as plain JDBC, so rxjava3-jdbc is confined to the streamed reads; a connection that cannot be leased is reported with the pool's message as the cause.
cportele
force-pushed
the
crud-writes-via-session
branch
from
September 2, 2026 13:12
baf0e42 to
8ae048a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes ldproxy/ldproxy#1761
Problem
CRUD writes (POST/PUT/PATCH/DELETE on items) ran through the rxjava3-jdbc transacted chain, which
returns the pooled connection only when its internal reference counter reaches zero. #590 covered
one way the counter never gets there (a later statement failing cancels the earlier ones). A second
way remained: when the COMMIT itself is rejected — a
DEFERRABLE INITIALLY DEFERREDconstraintor constraint trigger, or a serialization failure at commit under
SERIALIZABLE— the counter isalready zero, the library's follow-up rollback drives it to −1, and
close()never fires. Nothing islogged (the client gets a 400 "Invalid feature data"), HikariCP has no reason to evict, and every
such write costs one connection until the pool is exhausted:
A connection that dies mid-mutation is not affected: HikariCP evicts a connection on
connection-error SQLSTATEs (
08xxx,57P0x) regardless of what the application does afterwards.Change
FeatureProviderSql.createFeatures/updateFeature/deleteFeaturerun through the same JDBC sessionthe transactions module already uses (
openSession()→SqlMutationSessionoverJdbcSqlSession): one session per mutation, commit on success, rollback on error, connectionreturned in
finally. The generated statements are unchanged; PATCH keeps its delete-and-insertsemantics.
SqlClient.getMutationSource/getMutationFlow, thecreator/updater/deletion flows,
MutationTransactionGuard.from the pool directly (
HikariDataSource) instead of through the rxjava3-jdbcDatabase.JdbcSqlSession: a failed COMMIT finalises the session; the connection is released exactly once,with
setAutoCommit(true)andclose()isolated so a connection terminated by the database isstill returned.
08/53/57/58/XX,or unparsable JSON) stay a 400 with the existing message; a lost connection or an exhausted pool
is reported as a 500.
Behaviour changes worth noting
rejected feature leaves no rows behind.
Verification
JdbcSqlSessionSpecgains failed-COMMIT, failed-ROLLBACK and terminated-connection cases;SqlClientRxSpeccovers the lease paths; the former mutation-chain cases are gone with the codethey tested.
(statement rejected, backend terminated, COMMIT rejected via a deferred constraint trigger):
before the change every rejected COMMIT added one permanently active lease and two of them
starved a pool of size 2; after the change the pool stays at its baseline through repeated
failing COMMITs and killed backends, atomic collection POSTs leave no rows, and the
transaction smoke tests pass unchanged.
Relation to ldproxy/ldproxy#1645
#1645 asks for two things; this PR completes the second and the executor half of the first, and
leaves the PATCH-semantics half deliberately open:
transactions now run through the same
SqlMutationSession/JdbcSqlSession, with one transactionlifecycle and one connection-handling code path. What is not aligned, and stays open under
#1645, is the PATCH semantics: CRUD keeps its JSON Merge Patch applied as delete-and-insert of
the full feature, while the transactions module offers a property-level patch restricted to
configured non-object properties. Routing CRUD through that would change API behaviour, so it is
scheduled for 5.0.
chain and the transaction guard are gone, and connection leasing and statements without a result
no longer go through the library either.