Deferring transaction activation until DML is actually needed¶

1. Problem¶

A converted full-transaction block opens a real database transaction as soon as any record buffer for that database is touched — read or write. A great deal of converted code has the shape "open a full transaction, test a condition, write only if the condition holds", so the common path opens and commits a transaction that never wrote anything.

  DO TRANSACTION:                      /* TxWrapper created, inactive    */
     FIND FIRST customer NO-LOCK.      /* openScope -> activate() -> BEGIN */
     IF customer.balance > 0 THEN      /* ...condition false...          */
        ASSIGN customer.flag = YES.
  END.                                 /* COMMIT of a transaction that   */
                                       /* never wrote anything           */
  

The inactive-wrapper machinery already exists — TxWrapper.createTxWrapper creates wrappers that are not yet backed by a database transaction, and maybeActivateTxWrapper starts the real one later. Only the trigger is wrong. Today it fires from BufferManager.openScope (:2312), BufferManager.openScopeAt (:2658), RecordBuffer.initialize (:7778, :7923) and PersistenceInvocationHandler.invoke (:108).

The cost of a needless transaction is a BEGIN/COMMIT pair per block and — the expensive part on PostgreSQL — a snapshot held open for the whole duration of the block, holding back xmin and vacuum. Long read-only REPEAT loops are the worst case.

2. Proposed change¶

Move the activation trigger to first record mutation, per permanent database. The _temp database keeps the current buffer-scope trigger.

Two hooks, because one point is not sufficient:

• Primary (early). At the point a DMO transitions to "will be written" — record create, record delete, and the first field assignment — activate the wrapper for that DMO's database. This must run before BaseRecord's undoable-tracking call to Session.trackUndoable (BaseRecord.java:1849), because SavepointManager.trackUndoable (:485) asserts that the savepoint manager is already bound to a session.
• Backstop (late). In Persister — the single class that issues INSERT/UPDATE/DELETE SQL (insert, bulkInsert, bulkCopy, delete x2, update) — activate if not already active, and log when this path fires. The backstop firing means the primary hook missed a case; the log is how those cases are found, rather than shipping a silent write outside the transaction.

PersistenceInvocationHandler.invoke (the unsafe / direct-SQL proxy) keeps its unconditional activation — it cannot distinguish reads from writes.

3. Record lock bookkeeping must be decoupled from activation¶

Record locks are FWD-managed (LockManager), not database row locks. RecordLockContext.Perm defers release and downgrade while bufferManager.isTransaction() is true (RecordLockContext.java:504, :515), and the deferred release happens in RecordLockContext.transactionEnded() — called only from TxWrapper.end(), which TxWrapper.finished() skips for an inactive wrapper (TxWrapper.java:271-281).

Today that is unreachable: acquiring a lock requires a buffer, and opening a buffer scope activates the wrapper. After this change it becomes reachable — FIND ... EXCLUSIVE-LOCK with no subsequent write leaves the wrapper inactive and leaks the lock past the end of the transaction.

Proposed fix: split end() into a database half and a bookkeeping half. The bookkeeping half — RecordLockContext.transactionEnded() and the reclaimPendingKeys call that consumes its result — runs unconditionally in finished(), iterate() and retry(), whether or not the wrapper was ever activated. iterate() and retry() currently return early when inactive (TxWrapper.java:205-208, :227-230) and need the same treatment, otherwise an iterating read-only transaction block leaks locks at every iteration boundary.

4. Re-arming the deferral on iteration¶

TxWrapper.iterate() currently does end(); begin();. Once a REPEAT loop activates on one iteration, every later iteration carries a database transaction whether or not it writes. Since REPEAT with conditional DML is precisely the pattern under discussion, iterate() and retry() should instead deactivate: end the transaction, clear persistenceCtx, and return the wrapper to inactiveTxWrappers so the next iteration defers again. Session.commit already discards the savepoint manager, so re-installation on the next activate() is clean.

5. What does not change¶

TRANSACTION, _Trans, the TransactionManager transaction level and nesting, variable and temp-table UNDO, DirtyShareContext publication (there is nothing to publish without a mutation), and sequences (nextval is non-transactional on PostgreSQL regardless).

Reads in the pre-activation prefix run outside the transaction. Under READ COMMITTED that is a per-statement snapshot either way, so there is no visible difference; and an FWD session that intends to write holds an FWD exclusive lock, which already serialises FWD writers across the read-to-write window.

6. Configuration¶

persistence/defer-transaction-until-dml in the directory, read at bootstrap through Utils.getDirectoryNodeBoolean alongside force-no-undo-temp-tables (DatabaseManager.java:1789), defaulting to true. Setting it false restores the buffer-scope trigger, so a site that hits a regression reverts by configuration rather than by build.

7. Testing¶

A testcases battery covering: a read-only transaction block issues no BEGIN; conditional DML issues nothing on the false path and commits correctly on the true path; FIND ... EXCLUSIVE-LOCK with no write releases its lock at transaction end (the section 3 regression); nested sub-transaction blocks where the write happens in the innermost block; REPEAT writing on alternate iterations; UNDO both before and after the first DML; and multi-database blocks where only one database is written. Transaction counts asserted from TxWrapper instrumentation, lock state asserted cross-session through the two-session harness, and OpenEdge baselines captured for anything where 4GL semantics are in question.TxWrapper instrumentation, lock state asserted cross-session through the two-session harness, and OpenEdge baselines captured for anything where 4GL semantics are in question.

Risks¶

1. Silent write outside a transaction. This is the severe one. If a write path bypasses both the primary hook and the Persister backstop, it executes under autocommit and is committed immediately. UNDO then cannot roll it back and the failure is silent — no exception, just data that should not be there. The backstop and its logging exist specifically to bound this risk, but the risk is only as good as the enumeration of write paths.
2. Record lock leak past transaction end. Described in section 3. It is a genuine new regression introduced by the change, not a pre-existing one, and it is reachable from ordinary code (FIND ... EXCLUSIVE-LOCK with no write).
3. Ad-hoc transaction collision. Persistence.Context.beginTransaction returns false when a transaction is already open on the session, and TxWrapper.begin() throws IllegalStateException in that case unless initialTx is set. Deferring activation moves the window in which an ad-hoc transaction (for example the TEMP_CTX transactions in TemporaryBuffer, or a trigger touching another database) can be open when the master wrapper finally activates.
4. Error timing shift. A connection or transaction failure now surfaces at the first DML rather than at block entry. Code that expects to fail early — and any ON ERROR phrase scoped to catch it — sees the error at a different point in the block.
5. Read-then-write staleness against non-FWD writers. A record read before activation and updated after it was read outside the transaction. FWD's own lock manager serialises FWD sessions, but an external writer against the same database has a wider window than before.
6. Savepoint manager re-installation. Section 4 deactivates and reactivates wrappers across iterations. Session.beginTransaction throws if a savepoint manager is already installed, so the deactivation path has to leave the session genuinely clean; getting this subtly wrong produces an IllegalStateException only on loops that write on some iterations and not others.
7. Reduced observability. Anything that samples database transaction state — VSTs, monitoring, statement logs, support diagnostics — will report substantially fewer transactions. Existing dashboards and any tooling that infers activity from transaction counts will need to be re-read.

Open questions¶

1. Should section 4 (re-arming the deferral per iteration) land in this branch, or ship separately after sections 2, 3 and 6 have soaked? It carries the savepoint-manager risk above and is separable.
2. Is there any reason an EXCLUSIVE-LOCK read with no mutation needs a database transaction that I have not found? The lock manager is in-memory, but a dialect-specific escalation path would invalidate the premise.
3. Should the Persister backstop log, or fail fast in non-production builds? Failing fast finds missed write paths during testing; logging avoids turning a missed path into an abend at a customer.
4. Is the enumeration of write paths in section 2 complete — specifically, do audit record generation, database write triggers, and bulk/dynamic temp-table operations reach Persister, or do any of them reach the database another way?

Alex, please glance over the design and comment on the risks and open questions, thanks.

I am not sure whether Session.beginTransaction is something to be deferred at all. My point is that there is no BEGIN TRANSACTION SQL being run to the database, so starting a transaction even though no changes will be done is a no-op. My big concern is with the actual commit that is an actual SQL. I think delaying the transaction begin would be more complicated than using a flag for the first actual change done in that bracket.

I think the design is targeting an upper layer, including record locks iteration, savepoint manager. I don't think that any of these are relevant for the cause of simply avoiding the COMMIT SQL to save performance. Managing this at a higher level can help us allocate less memory resources (TxWrapper instances and so on), but the optimization will be marginal.

I would like to scope this to the orm package and don't let the user (i.e. 4GL compatibility layer) plug into it.

A phase 1 of this task will be to abstract any JDBC connection through session if possible. Thus, Session will always know the database state at a moment of time. Arbitrary SQL run by direct-Java access is accounted as "drop any optimization chances".

A phase 2 would be to hook in the right API of the Session to detect if the ORM session actually changed the DB in any way, in order to make commit a no-op eventually.

Please update % done based on where you estimate the development of this solution is currently.

Review of 11884a / r16764

• SavepointManager.java
  • line 56: 017 is the correct H entry number; Please update the one from line 55 to 16;
• Session.java
  • lines 469-470: two extra empty lines;
  • lines 1857: the savepoints should be stacked if the transaction was not materialised. Consider the following code:

    do transaction:                   // begin transaction, no materialisation
       do transaction:                // set savepoint, materialisation forces transaction to materialise
          if (false-condition)
             bufferX.fieldY = 10.     // never executes, leaving the transaction 'empty'
       end.                           // release savepoint
    
       // another non-empty sub-transaction may exist here which could materialise the transaction
    end.                              // commit of empty transaction

    The difficulty comes the fact that this method returns a Savepoint. But it can be a 'lazy' one.
  • lines 1975, 1945, see above. If the materialisation did not happen, the lazy savepoint proxy should be dropped from the stack;
  • lines 2544-2573: package-private method prepareForStreaming() should be moved from private section up the file;
• SQLQuery.java
  • line 1343: (existing error in trunk (since 2023) noticed while looking at the changes, but please fix it) this method (resetErrorHandler()) should delegate to Dialect.resetErrorHandler(), not Dialect.activateErrorHandler() - probably a copy/paste error.

• in BlobType.readProperty() and ClobType.readProperty(), the connection is always acquired using getConnection() which will cause the transaction to materialise, only to setAutoCommit(false) immediately. This should happen conditionally (if (isAutoCommit) ...). Of course, the restore should also be conditioned.

Tested #10275, the commit statements were reduced 10x, from ~340k to ~34k. Also did some regression testing with the multi tenant app, it passed.

AI-based review:

• [MAJOR] functional Session.commit: commit now restores auto-commit via resetAutoCommit, which breaks the ANALYZE step of ImportWorker.run on the PostgreSQL bulk-copy path (the default import path, since only P2JPostgreSQLDialect supportsBulkCopy). The import session does beginTransaction / bulkCopy / commit (ImportWorker ~1747-1783) and stays open; the ANALYZE block (~2073-2087) takes session.getConnection() (no materialization, inTx is false), runs analyze <table> and calls connection.commit() directly, which pgjdbc 42.7.8 rejects with "Cannot commit when autoCommit is enabled."; the catch's connection.rollback() throws likewise and escapes to the outer catch (Exception) (~2100). Every non-empty table is logged SEVERE "Error processing import data" and added to fatal although its rows were committed. Previously the connection stayed in manual-commit mode after commit, so this worked. Fix in ImportWorker (skip commit/rollback when getAutoCommit() is true, or bracket ANALYZE with session.beginTransaction() / session.commit()).

DB import thus should be regression tested, as well as other processes that we have external to the runtime that involve the DB.

• [MAJOR] performance BlobType.readProperty: (also ClobType.readProperty) Previously a (reclaimed, long-lived) session's connection stayed out of auto-commit after its first transaction, so the setAutoCommit(false) / restore around the LOB read were driver no-ops. Now resetAutoCommit restores auto-commit after every commit/rollback and deferral keeps it on until first DML, so in the window after a commit/rollback until the next DML, savepoint or forward-only query, every non-streaming read (SQLQuery.list/uniqueResult, paged list, Loader, scroll-insensitive) of a BLOB/CLOB field toggles the mode per LOB cell per row: two round trips per cell on SQL Server (set implicit_transactions) and MariaDB (set autocommit), one extra COMMIT per non-null blob on PostgreSQL. E.g. FIND tbl ... NO-LOCK or a 100-row preselect list with two LOB columns (~400 extra round trips on SQL Server/MariaDB).

To be investigated.

• [MINOR] performance Session.commit: (also rollback, via clearAutoCommit/resetAutoCommit) every transaction that materializes now flips auto-commit off and back on; on SQL Server (mssql-jdbc 12.10) and MariaDB (Connector/J 3.5.3) each flip is a server round trip, so a materialized transaction costs 3 round trips instead of 1 (pgjdbc flips locally, no extra cost). Any DML materializes; on MariaDB a non-scrolling FOR EACH also does via prepareForStreaming (on SQL Server supportsJdbcApi(API_LAST_ON_FORWARD_ONLY) is false so ordinary FOR EACH does not stream). The deferral pays off on these dialects only if fewer than about one in three transactions materialize; worth measuring before enabling defer-transaction-until-dml by default there.
• [MINOR] functional BlobType.readProperty: (also ClobType.readProperty) the new comment says the guard is needed "because restoring it would commit whatever that transaction was in the middle of". That is incorrect: with auto-commit already off the old restore was setAutoCommit(false), a no-op per JDBC and on pgjdbc, mssql-jdbc, MariaDB Connector/J and H2 (all return early on an unchanged mode). Reword the comment: the guard only avoids a redundant driver call.

I see many of the changes are around the setAutoCommit(false) logic. I am not sure if this is something that is still relevant for FWD.

As long as FWD uses a connection pool, once setAutoCommit(false) is used, that connection will always be auto-commit false, even when checked-out. In other words, as long as we don't connect to the database with auto-commit set, this setAutoCommit(false) seems to be just a "lazy initialization" of the connection. In most cases, this is redundant as the connection is already non-auto-commit (e.g. when checked out of c3p0).

Artur, please document the use-cases of FWD when auto-commit is desired to be true. I am inclined to make false default and enable it when needed instead of enabling it by default and always taking care to turn it off. Something like a c3p0 configuration "on connection established" to set it to auto-commit false always.

Artur Școlnic wrote:

we don't rely on the autocommit being true,

• either we set autocommit to true deliberatelly.
• or we get a connection and don't do anything with the autocommit -> this presumes that it will use the true default.
  *this is buggy, because once another session checks out the connection, sets it autocommit to false and checks it back in, then the "default" becomes false for *some transactions.

Alex, to answer your initial question, the autocommit is largely not used in FWD, but it is essential for this branch.
Trunk used one boolean for two different things. The branch splits them:
- inTx — a transaction has been requested. Unchanged timing; still what TRANSACTION returns.
- txMaterialized — the JDBC connection has actually left auto-commit.

Lifecycle

1. beginTransaction sets inTx and touches no JDBC state. The connection stays in auto-commit.
2. Reads in the prefix are therefore implicit single-statement transactions — nothing held open, nothing owed.
3. The first write reaches checkTx() → clearAutoCommit(), the single place the connection leaves auto-commit and the point txMaterialized is set. The other materialization points are setSavepoint (a savepoint cannot exist without a transaction behind it), getConnection() (caller's intent unknown), and prepareForStreaming.
4. commit() / rollback() skip the SQL entirely when txMaterialized == false, then resetAutoCommit() restores auto-commit so the next transaction on that session can defer in its turn.

Lifecycle

Completely agreed. This was my understanding as well on your changes.
But, I don't understand why we need to so much work around the auto-commit when it is not used by FWD. I prefer to remove it entirely (and move it to c3p0 connection setup time). This way, the changes in #11884 won't need to deal with auto-commit at all.

Alex, to answer your initial question, the autocommit is largely not used in FWD, but it is essential for this branch.

I don't see where this "essential" is. What would happen if 11884a changes will not touch auto-commit at all?

Alexandru Lungu wrote:

I don't see where this "essential" is. What would happen if 11884a changes will not touch auto-commit at all?

The saving is "don't send COMMIT", and that is only safe if no transaction was ever opened, in JDBC, "no transaction is open" is auto-commit mode. So the auto-commit handling isn't wrapping around the optimization; it is the optimization.

The saving is "don't send COMMIT", and that is only safe if no transaction was ever opened,

I agree

in JDBC, "no transaction is open" is auto-commit mode.

I am not sure what you mean by this.

• If auto-commit is set to true, a transaction is open if you send BEGIN TRANSACTION and ends on COMMIT / ROLLBACK
• If auto-commit is set to false, a transaction is open automatically at first DML.

So the auto-commit handling isn't wrapping around the optimization; it is the optimization.

There is a decision to be made if FWD uses either explicit BEGIN in auto-commit true or implicit BEGIN on first DML in auto-commit false.
As I am inclined to say the second is the one adopted by FWD, I can't tell why we need to set auto-commit for each transaction, instead of only once at the connection set-up.

Alexandru Lungu wrote:

• If auto-commit is set to true, a transaction is open if you send BEGIN TRANSACTION and ends on COMMIT / ROLLBACK
• If auto-commit is set to false, a transaction is open automatically at first DML.

With auto-commit false, PostgreSQL opens the transaction at the first statement, not the first DML. I could retest, but this was my conclusion from prior probes.

With auto-commit false, PostgreSQL opens the transaction at the first statement, not the first DML.

Now that I think about it, does it even matter? Even if the transaction is scoped larger, as long as there is no DML, the rollback/commit is a no-op for these.

I could retest, but this was my conclusion from prior probes.

I am not sure how you can test this considering the mention above. If you rollback, all DMLs will be reverted. However, how could you know if the queries done before the DMLs were captured by the transaction as well.

The transaction starts at the DML because FWD toggles the flag there. Nothing about auto-commit=false defers to DML; it opens on the first statement of any kind. FWD gets DML timing by staying in auto-commit until the write, then switching.

Now that I think about it, does it even matter? Even if the transaction is scoped larger, as long as there is no DML, the rollback/commit is a no-op for these.

Maybe it does if the connection is question does only queries. In this case, no COMMIT will occur so the transaction will be left opened. But this also happens in trunk right? Unless we start it in auto-commit true and on first DML we switch it to auto-commit false till the commit. After commit, we should switch it back to auto-commit true so that upcoming queries do not start long running transactions.

-         Query.getDialect(session).activateErrorHandler(session.getConnection());
+         Query.getDialect(session).resetErrorHandler(session.getConnectionForRead());

• This looks like an unrelated change.

Otherwise, I am ok with the changes

Alexandru Lungu wrote:

• This looks like an unrelated change.

It's a quick fix requested in note 10, but yes, unrelated.

The following testing was done and passed:
Multi tenant app - harness, reports, webui navigation, convlaunvch
ETF - full testing suite
Chui regression tests
Large gui app unit tests

the results with trunk are the same.

I think we can merge 11884a.

I think we can merge 11884a.

Please merge 11884a to trunk now.
Afterwards, please assess each customer checking whether they use direct Java access. If they do, open a separate task on their project to suggest changes to their native Java code or at least notify through that task this change.