Skip to content

DBExecutor commits individual operations inside performInTransaction, preventing rollback #2

Description

@vinit-thummar

Hi Philip,

While testing concurrent metadata lookups and endpoint updates in phoss SMP 8.4.1, I found a transaction-boundary problem in ph-db-jdbc 8.4.3. I reproduced it independently of SMP using H2.

Observed behaviour

performInTransaction routes SQL operations through withExistingConnectionDo, which calls JDBCHelper.commit after each operation.

This commits changes before the enclosing transaction callback finishes. A subsequent exception causes the transaction to report failure, but its rollback cannot undo the already committed changes.

Relevant source: DBExecutor.java in 8.4.3.

Reproduction

Tested with:

  • ph-db-jdbc 8.4.3
  • H2 2.4.240
  • Writer connection with autoCommit=false
  • A separate observer connection using READ_COMMITTED
  • A table containing one committed row, id=1

The essential operation is:

executor.performInTransaction (() -> {
  executor.insertOrUpdateOrDelete (
      "DELETE FROM probe_metadata WHERE id=?",
      new ConstantPreparedStatementDataProvider (Integer.valueOf (1)));

  // SELECT COUNT(*) from the observer connection returns 0 here.
  // Expected: 1, because this transaction has not committed.

  throw new SQLException ("Deliberate failure to check rollback");
});

Results from the two-connection test:

Check Direct JDBC control DBExecutor
Observer row count before transaction completion 1 0
Observer row count after rollback 1 0

performInTransaction returns FAILURE, but the deleted row remains missing. The direct JDBC control uses the same connection settings and rolls back correctly.

Impact observed in SMP

With SMP 8.4.1's SQL backend, I tested 50 metadata GETs/second against a 1,000-participant fixture while updating an existing endpoint approximately once per second.

Two 30-second runs produced 6 and 4 unexpected HTTP 404 responses. All updates succeeded, and every 404 overlapped an update of that participant. Comparable read-only runs had no HTTP failures.

This is consistent with intermediate deletion becoming visible during an update. I would rerun the SMP scenario after a library fix to confirm that it eliminates these failures.

Proposed contribution

Would you be open to a focused PR that keeps commit/rollback handling at the enclosing transaction boundary while preserving standalone operation behaviour?

I would include deterministic tests for successful commit, visibility before commit, rollback after failure, and nested transactions, followed by validation in SMP.

Thanks!

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions