The GitHub Actions job "Pull Request" on openjpa.git/OPENJPA-2951 has succeeded. Run started by GitHub user rzo1 (triggered by rzo1).
Head commit for run: 68122abb6e9f4aacbfe742662c00672d9f9b14f4 / Richard Zowalla <[email protected]> [OPENJPA-2951] Make the context flush after an in-memory bulk operation escapable A bulk UPDATE or DELETE that cannot be expressed as a single statement is executed in memory: OpenJPA loads the matching instances and applies the operation to them, so the result is a modified persistence context, not a modified database. The flush added for OPENJPA-2940 therefore carries the row count that executeUpdate() reports to the database - without it an em.clear() before the commit discards the operation while the count has already been reported, and a find() after that clear still sees the old row. Measured on Derby with SQL logging: with the flush an in-memory delete emits the owned-table and entity DELETE statements during executeUpdate() and a find() after em.clear() returns null; without it only the candidate SELECT is emitted and find() returns the instance again. The flush must not turn a legal bulk operation into a failure, though. Broker.flush() rejects an incremental flush when the runtime does not support OPTION_INC_FLUSH - which AbstractStoreManager.getUnsupportedOptions() does by default, so openjpa-xmlstore raised "This data store does not support incremental flushing" for every bulk delete - and when a savepoint is set that the SavepointManager cannot flush incrementally, which the default in-mem manager cannot. Both rejections happen before anything is written, but the persistence layer marks the transaction rollback-only on the way out, so the whole transaction was lost where 4.1.x simply applied the operation. The new Broker.isFlushSupported() reports both conditions, and the bulk operation now skips the flush and logs it at INFO instead of flushing blindly. The pre-flush of the candidate query has always been guarded by OPTION_INC_FLUSH in the same way. The second cost is that the kernel has no way to flush single instances. BrokerImpl.flush(int) is protected, always operates on getTransactionalStates(), fires the flush events with that whole collection, calls afterFlush() on every state and clears FLAG_FLUSH_REQUIRED, and the Broker interface exposes only flush() and preFlush(); flushing the affected instances alone would need more kernel API, which is left as a follow-up. So unrelated pending changes are written as well. Under the default FlushModeType.AUTO that is usually not a change of its own, because the query that loads the candidates flushes the context first - but that pre-flush of unrelated changes is itself new in 4.2.0 (OPENJPA-2940), and it does not happen at all when the query never reaches the datastore, i.e. when the candidates were handed over with setCandidateCollection(): isInMemory() short-circuits the pre-flush before it looks at the flush mode. The trailing flush is then the only flush even under AUTO, which the SQL log confirms - the candidate SELECT precedes the UPDATE of the unrelated instance. The other case is FlushModeType.COMMIT, or FlushBeforeQueries=false or with-connection without an open connection, where a dirty instance of a type that is not on the query's access path used to stay pending. IgnoreChanges is not involved: QueryImpl line 1032 short-circuits it for every operation other than OP_SELECT. The flush also decides when the AFTER_DELETE_PERFORMED and AFTER_UPDATE_PERFORMED callbacks of the affected instances fire, since StateManagerImpl.afterFlush() raises them. That is the route most applications take into this code: JDBCStoreQuery.executeDelete() gives up on the single statement precisely when the candidate has delete listeners, "so the callbacks are invoked". Neither behaviour is mandated. Jakarta Persistence 3.1 section 4.10 states that "the persistence context is not synchronized with the result of the bulk update or delete" and recommends that "bulk update and delete operations should only be performed within a transaction in a new persistence context or before fetching or accessing entities whose state might be affected by such operations". A reported row count that never reached the database is the worse of the two, so the flush stays the default and becomes escapable through the compatibility option FlushAfterInMemoryBulkOperation. Suppressing the flush must not and does not suppress the in-memory application of the operation: the instances are still modified or marked for deletion and are written by the next flush or at commit. The new section of the migration considerations documents the flush, the pending changes and callbacks it moves, the two cases in which it is the only flush, the option and the fallback when the context cannot be flushed, which covers the flush after the operation only - the flush before the candidate query is not guarded against a savepoint, so an operation issued with pending changes and a savepoint set still fails as it did before. No test in the tree depended on the flush - the bulk and criteria suites all pass with it removed. Six of the twelve new methods in openjpa-persistence-jdbc fail with the kernel part of this change reverted; the other six pin behaviour OPENJPA-2940 already introduced and pass on master as well. TestInMemoryBulkFlush pins the default, including the candidate-collection case under AUTO and the savepoint fallback on a clean context; four of its six methods assert the candidate SELECT so they cannot pass on a datastore-executed operation, and the other two assert the presence or absence of a single statement; TestInMemoryBulkFlushCompat pins the option; TestInMemoryBulkFlushCallbacks pins the callback timing on both paths; and TestPersistence of openjpa-xmlstore covers the AbstractStoreManager default, which had no bulk-operation test at all. Report URL: https://github.com/apache/openjpa/actions/runs/39151363388 With regards, GitHub Actions via GitBox
