Incomplete SQL transaction in create()
Description
Environment
Activity
Rod WiddowsonMarch 24, 2025 at 10:28 AM
Reopen to set fix in
Rod WiddowsonSeptember 24, 2024 at 7:19 PM
@Tom Zeller You might want to either rebuild or try with tomorrows snapshot. Thanks for the report
Rod WiddowsonSeptember 24, 2024 at 7:15 PMEdited
>You mean by turning off auto-commit? Might work, but this is definitely pushing into crazy territory.
Na simpler than that. Remove all the spurious rollbacks that I added (belt & suspenders) with the last.
I’ve also added some asserts to the connection to make sure that we do exactly one of autoCommit/commit/rollback and that works nicely because tts not Db depdendant
It also speaks to your “call commit” suggestion because that's exactly what auto commit does: “Do a commit/stop transaction/start transaction after every operation.”
The asserts worked (I found a bug) so I am about to push
Scott CantorSeptember 24, 2024 at 7:12 PM
Unless you mean my suggestion about just turning off auto-commit…then yeah, makes sense to rollback if it’s not already been committed, but I’m not sure how you could tell, maybe a flag on the wrapper object…
Scott CantorSeptember 24, 2024 at 7:11 PM
I’ll review and change. I’m actually almost tempted to make it automatic (so if an auto-commit connection is closed without being committed we roll it back)
You mean by turning off auto-commit? Might work, but this is definitely pushing into crazy territory.
I’m really uncomfortable doing this its like saying to the App “No, nothing happened” and saying to the database “Write it all”. This is fine for as long as we know we haven’t written anything and I agree its unlilky that we will change the code path, but its precisely that unlikeliness which makes it risky
These aren’t generic code paths though, it’s in the logic implementing the reads and writes, so we know exactly what we did or didn’t do, and nothing we do in this code ever needs to be rolled back. If we were writing some general database layer, probably different story.
We probably need to review all this in the persistent ID code and the data connector BTW.
When
create()exits in the conditional blockif (returnedExpiration == null || System.currentTimeMillis() < returnedExpiration) { log.debug("Duplicate record '{}' in context '{}'", key, context); return false; }the SQL connection is returned to the pool with the transaction still open. The next time this connection is retrieved from the pool by the constructor of
ConnectionWithLock, the call tosetTransactionIsolation()will fail withorg.postgresql.util.PSQLException: Cannot change transaction isolation level in the middle of a transaction.if the PostgreSQL JDBC driver is used.