codeconsole commented on code in PR #15744: URL: https://github.com/apache/grails-core/pull/15744#discussion_r3502800187
########## grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoTransaction.java: ########## @@ -0,0 +1,165 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.datastore.mapping.mongo; + +import com.mongodb.MongoException; +import com.mongodb.client.ClientSession; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.grails.datastore.mapping.transactions.Transaction; + +/** + * A {@link Transaction} backed by a real MongoDB multi-document transaction on a + * {@link ClientSession}. Unlike the legacy + * {@link org.grails.datastore.mapping.transactions.SessionOnlyTransaction} (which only flushes the + * GORM session), this commits or aborts a server-side transaction so multiple writes are atomic. + * + * <p>The {@link ClientSession} is started with an active transaction before this object is + * constructed; {@link #commit()} flushes the GORM session (a no-op when the + * {@link org.grails.datastore.mapping.transactions.DatastoreTransactionManager} already flushed) + * and commits the server transaction, while {@link #rollback()} aborts it. Both close the + * {@link ClientSession} and detach it from the owning session.</p> + * + * @since 8.0 + */ +public class MongoTransaction implements Transaction<ClientSession> { + + /** + * Maximum number of times {@link ClientSession#commitTransaction()} is retried when the server + * reports an {@code UnknownTransactionCommitResult} (i.e. the commit outcome is unknown and the + * operation is safe to retry). + */ + private static final int MAX_COMMIT_RETRIES = 3; + + private static final Logger LOG = LoggerFactory.getLogger(MongoTransaction.class); + + private static volatile boolean warnedTimeoutIgnored = false; + + private final AbstractMongoSession session; + private final ClientSession clientSession; + private boolean active = true; + + public MongoTransaction(AbstractMongoSession session, ClientSession clientSession) { + this.session = session; + this.clientSession = clientSession; + } + + @Override + public void commit() { + if (!active) { + return; + } + boolean committed = false; + try { + // Flush pending GORM operations into the active transaction. When driven by the + // DatastoreTransactionManager the session was already flushed, so this clears nothing + // and is a no-op; it covers callers that commit the transaction directly. + session.flush(); + commitWithRetry(); + committed = true; + } finally { + if (!committed) { + // The commit (or the flush before it) failed. Explicitly abort the server transaction + // rather than relying on close() to do so implicitly, then discard the GORM session's + // pending operations and first-level cache so a reused session cannot return entities + // that were never committed. + if (clientSession.hasActiveTransaction()) { + try { + clientSession.abortTransaction(); + } + catch (RuntimeException e) { + LOG.debug("Error aborting transaction after failed commit: {}", e.getMessage(), e); + } + } + try { + session.clear(); + } + catch (RuntimeException e) { + LOG.debug("Error clearing session after failed transaction commit: {}", e.getMessage(), e); + } + } + close(); + } + } + + @Override + public void rollback() { + if (!active) { + return; + } + try { + if (clientSession.hasActiveTransaction()) { + clientSession.abortTransaction(); + } + } finally { + close(); + } + } + + @Override + public ClientSession getNativeTransaction() { + return clientSession; + } + + @Override + public boolean isActive() { + return active; + } + + @Override + public void setTimeout(int timeout) { + // The transaction is started before the manager applies a timeout, so a per-transaction + // timeout cannot be applied to the server-side transaction; the server enforces its own + // transactionLifetimeLimitSeconds instead. Warn once so a configured timeout is not silently + // ignored. + if (!warnedTimeoutIgnored) { + warnedTimeoutIgnored = true; + LOG.warn("A per-transaction timeout was requested but GORM for MongoDB does not apply it to the " + Review Comment: Done — throws `TransactionUsageException` on a non-default timeout now. `doBegin` rolls back the started session and rethrows as `CannotCreateTransactionException`, so nothing leaks. ########## grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/AbstractMongoSession.java: ########## @@ -200,6 +215,120 @@ public MongoMappingContext getMappingContext() { return (MongoMappingContext) super.getMappingContext(); } + /** + * @return the active {@link ClientSession} for the current MongoDB transaction, or {@code null} + * if no server-side transaction is in progress + */ + public ClientSession getClientSession() { + return clientSession; + } + + /** + * @return {@code true} if a server-side MongoDB transaction is currently active on this session + */ + public boolean hasActiveTransaction() { + return clientSession != null && clientSession.hasActiveTransaction(); + } + + /** + * Detaches the {@link ClientSession} from this session once its transaction has completed. + * Called by {@link MongoTransaction} after commit or rollback closes the session. + */ + void clearClientSession() { + this.clientSession = null; + } + + /** + * Closes and detaches the {@link ClientSession} if one is still attached. Used defensively when a + * transaction did not complete through {@link MongoTransaction}, so a session is never leaked. + */ + protected void closeClientSessionQuietly() { + if (clientSession != null) { + try { + clientSession.close(); + } + catch (RuntimeException ignored) { + // best effort + } + finally { + clientSession = null; + } + } + } + + @Override + public void disconnect() { + try { + closeClientSessionQuietly(); + } + finally { + super.disconnect(); + } + } + + @Override + protected Transaction beginTransactionInternal() { + if (getDatastore().isTransactionsEnabled()) { + // Defensive: if a previous transaction did not complete cleanly, close its orphaned + // session before starting a new one so it cannot leak. + closeClientSessionQuietly(); + ClientSession session = getNativeInterface().startSession(); + try { + session.startTransaction(); + } + catch (RuntimeException e) { + session.close(); + throw e; + } + this.clientSession = session; + return new MongoTransaction(this, session); + } + return new SessionOnlyTransaction<>(getNativeInterface(), this); + } + + // The driver exposes a session-less and a ClientSession overload for every operation, and the + // session argument cannot be null. These helpers branch once so call sites stay readable and + // behave identically (session-less) when no transaction is active. + + @SuppressWarnings({"rawtypes", "unchecked"}) + public BulkWriteResult bulkWrite(com.mongodb.client.MongoCollection collection, List<? extends WriteModel> writes) { + return clientSession != null ? collection.bulkWrite(clientSession, writes) : collection.bulkWrite(writes); + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + public DeleteResult deleteMany(com.mongodb.client.MongoCollection collection, Bson filter) { + return clientSession != null ? collection.deleteMany(clientSession, filter) : collection.deleteMany(filter); Review Comment: Yes — all nine use `hasActiveTransaction()` now. Only the transactional case needs the session, and it falls back to the session-less overload if one ever outlives its transaction. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
