From cccb3b95e861d0ebf733320ef7b6c6ee323b354d Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 17 Oct 2024 11:50:23 -0400 Subject: [PATCH 1/6] only force majority write concern on commitTransaction retry --- src/sessions.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/sessions.ts b/src/sessions.ts index bad966ed71c..a4a9984a43f 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -124,6 +124,7 @@ export class ClientSession owner?: symbol | AbstractCursor; defaultTransactionOptions: TransactionOptions; transaction: Transaction; + commitAttempted?: boolean; /** @internal */ [kServerSession]: ServerSession | null; /** @internal */ @@ -417,6 +418,7 @@ export class ClientSession ); } + this.commitAttempted = false; // increment txnNumber this.incrementTransactionNumber(); // create transaction state @@ -474,7 +476,7 @@ export class ClientSession WriteConcern.apply(command, { wtimeoutMS: 10000, w: 'majority', ...wc }); } - if (this.transaction.state === TxnState.TRANSACTION_COMMITTED) { + if (this.transaction.state === TxnState.TRANSACTION_COMMITTED || this.commitAttempted) { WriteConcern.apply(command, { wtimeoutMS: 10000, ...wc, w: 'majority' }); } @@ -494,8 +496,10 @@ export class ClientSession try { await executeOperation(this.client, operation); + this.commitAttempted = undefined; return; } catch (firstCommitError) { + this.commitAttempted = true; if (firstCommitError instanceof MongoError && isRetryableWriteError(firstCommitError)) { // SPEC-1185: apply majority write concern when retrying commitTransaction WriteConcern.apply(command, { wtimeoutMS: 10000, ...wc, w: 'majority' }); @@ -503,7 +507,14 @@ export class ClientSession this.unpin({ force: true }); try { - await executeOperation(this.client, operation); + await executeOperation( + this.client, + new RunAdminCommandOperation(command, { + session: this, + readPreference: ReadPreference.primary, + bypassPinningCheck: true + }) + ); return; } catch (retryCommitError) { // If the retry failed, we process that error instead of the original From 2411c43da44503293f75d21090381ca051655dbf Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 17 Oct 2024 11:51:03 -0400 Subject: [PATCH 2/6] add int test --- .../transactions/transactions.test.ts | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/test/integration/transactions/transactions.test.ts b/test/integration/transactions/transactions.test.ts index 7e22d65d209..bc604a28168 100644 --- a/test/integration/transactions/transactions.test.ts +++ b/test/integration/transactions/transactions.test.ts @@ -3,6 +3,7 @@ import { expect } from 'chai'; import { ClientSession, type Collection, + type CommandStartedEvent, type MongoClient, MongoInvalidArgumentError, MongoNetworkError, @@ -231,8 +232,14 @@ describe('Transactions', function () { context('when completing a transaction', () => { let client: MongoClient; + let commandsStarted: CommandStartedEvent[]; beforeEach(async function () { - client = this.configuration.newClient(); + client = this.configuration.newClient(undefined, { monitorCommands: true }); + commandsStarted = []; + client.on('commandStarted', ev => { + console.log(ev); + commandsStarted.push(ev); + }); }); afterEach(async function () { @@ -260,6 +267,25 @@ describe('Transactions', function () { }) ) ); + + it( + 'commitTransaction does not override write concern on initial attempt', + { requires: { mongodb: '>=4.2.0', topology: '!single' } }, + async function () { + const session = client.startSession({ + defaultTransactionOptions: { writeConcern: { w: 1 } } + }); + session.startTransaction(); + await client.db('test').collection('test').insertOne({ x: 1 }, { session }); + await session.commitTransaction(); + + const commitTransactions = commandsStarted.filter( + x => x.commandName === 'commitTransaction' + ); + expect(commitTransactions).to.have.lengthOf(1); + expect(commitTransactions[0].command).to.have.nested.property('writeConcern.w', 1); + } + ); }); describe('TransientTransactionError', function () { From ca48b611ad6138231afde915462ce36257af6462 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 17 Oct 2024 12:06:51 -0400 Subject: [PATCH 3/6] add doc comment --- src/sessions.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sessions.ts b/src/sessions.ts index a4a9984a43f..4029744dcac 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -124,6 +124,9 @@ export class ClientSession owner?: symbol | AbstractCursor; defaultTransactionOptions: TransactionOptions; transaction: Transaction; + /** @internal + * Keeps track of whether or not the current transaction has attempted to be committed. Is + * initially undefined. Gets set to false when startTransaction is called. When commitTransaction is sent to server, if the commitTransaction succeeds, it is then set to undefined, otherwise, set to true */ commitAttempted?: boolean; /** @internal */ [kServerSession]: ServerSession | null; From 3c8707e472ef9361057f4c036679190914f07aec Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 17 Oct 2024 15:24:34 -0400 Subject: [PATCH 4/6] pre-create collection --- test/integration/transactions/transactions.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/transactions/transactions.test.ts b/test/integration/transactions/transactions.test.ts index bc604a28168..27c4915208c 100644 --- a/test/integration/transactions/transactions.test.ts +++ b/test/integration/transactions/transactions.test.ts @@ -272,11 +272,12 @@ describe('Transactions', function () { 'commitTransaction does not override write concern on initial attempt', { requires: { mongodb: '>=4.2.0', topology: '!single' } }, async function () { + const collection = await client.db('test').createCollection('test'); const session = client.startSession({ defaultTransactionOptions: { writeConcern: { w: 1 } } }); session.startTransaction(); - await client.db('test').collection('test').insertOne({ x: 1 }, { session }); + await collection.insertOne({ x: 1 }, { session }); await session.commitTransaction(); const commitTransactions = commandsStarted.filter( From 79967fcd2138d64d3e3938afac4ffc0b11bfa640 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 17 Oct 2024 15:27:48 -0400 Subject: [PATCH 5/6] pre-drop collection --- test/integration/transactions/transactions.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/integration/transactions/transactions.test.ts b/test/integration/transactions/transactions.test.ts index 27c4915208c..84e64b76f25 100644 --- a/test/integration/transactions/transactions.test.ts +++ b/test/integration/transactions/transactions.test.ts @@ -272,6 +272,10 @@ describe('Transactions', function () { 'commitTransaction does not override write concern on initial attempt', { requires: { mongodb: '>=4.2.0', topology: '!single' } }, async function () { + await client + .db('test') + .dropCollection('test') + .catch(() => null); const collection = await client.db('test').createCollection('test'); const session = client.startSession({ defaultTransactionOptions: { writeConcern: { w: 1 } } From bf4b60d2966f63225a2ceac3aac7264cf01768d3 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 17 Oct 2024 16:55:28 -0400 Subject: [PATCH 6/6] remove log --- test/integration/transactions/transactions.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/transactions/transactions.test.ts b/test/integration/transactions/transactions.test.ts index 84e64b76f25..b0ecadecb88 100644 --- a/test/integration/transactions/transactions.test.ts +++ b/test/integration/transactions/transactions.test.ts @@ -237,7 +237,6 @@ describe('Transactions', function () { client = this.configuration.newClient(undefined, { monitorCommands: true }); commandsStarted = []; client.on('commandStarted', ev => { - console.log(ev); commandsStarted.push(ev); }); });