diff --git a/packages/compass-crud/src/stores/crud-store.js b/packages/compass-crud/src/stores/crud-store.js index 6484b9bea01..571b1b332e5 100644 --- a/packages/compass-crud/src/stores/crud-store.js +++ b/packages/compass-crud/src/stores/crud-store.js @@ -183,7 +183,7 @@ const configureStore = (options = {}) => { ns: '', collection: '', abortController: null, - session: null, + sessions: null, error: null, docs: [], start: 0, @@ -613,7 +613,7 @@ const configureStore = (options = {}) => { this.setState({ status: DOCUMENTS_STATUS_FETCHING, abortController, - session, + sessions: [session], error: null }); @@ -639,7 +639,7 @@ const configureStore = (options = {}) => { table: this.getInitialTableState(), resultId: resultId(), abortController: null, - session: null + sessions: null }); abortController.signal.removeEventListener('abort', this.onAbort); this.localAppRegistry.emit('documents-paginated', view, documents); @@ -1002,14 +1002,16 @@ const configureStore = (options = {}) => { } const fetchShardingKeysOptions = { - maxTimeMS: query.maxTimeMS + maxTimeMS: query.maxTimeMS, + session: this.dataService.startSession() }; const countOptions = { skip: query.skip, maxTimeMS: query.maxTimeMS > COUNT_MAX_TIME_MS_CAP ? COUNT_MAX_TIME_MS_CAP : - query.maxTimeMS + query.maxTimeMS, + session: this.dataService.startSession() }; if (this.isCountHintSafe()) { @@ -1023,7 +1025,8 @@ const configureStore = (options = {}) => { limit: NUM_PAGE_DOCS, collation: query.collation, maxTimeMS: query.maxTimeMS, - promoteValues: false + promoteValues: false, + session: this.dataService.startSession() }; // only set limit if it's > 0, read-only views cannot handle 0 limit. @@ -1039,7 +1042,6 @@ const configureStore = (options = {}) => { countOptions }); - const session = this.dataService.startSession(); const abortController = new AbortController(); const signal = abortController.signal; @@ -1051,12 +1053,6 @@ const configureStore = (options = {}) => { countOptions.signal = signal; findOptions.signal = signal; - // pass the session so that the queries are all associated with the same - // session and then we can kill the whole session once - fetchShardingKeysOptions.session = session; - countOptions.session = session; - findOptions.session = session; - // Don't wait for the count to finish. Set the result asynchronously. countDocuments(this.dataService, ns, query.filter, countOptions) .then((count) => this.setState({ count, loadingCount: false })) @@ -1079,7 +1075,21 @@ const configureStore = (options = {}) => { this.setState({ status: DOCUMENTS_STATUS_FETCHING, abortController, - session, + /** + * We have separate sessions created for the commands we are running as + * running commands with the same session concurrently is not really + * supported by the server. Even though it works in some environments, + * it breaks in others, so having separate sessions is a more spec + * compliant way of doing this + * + * @see https://docs.mongodb.com/manual/core/read-isolation-consistency-recency/#client-sessions-and-causal-consistency-guarantees + * @see https://github.com/mongodb/specifications/blob/master/source/sessions/driver-sessions.rst#why-do-we-say-drivers-must-not-attempt-to-detect-unsafe-multi-threaded-or-multi-process-use-of-clientsession + */ + sessions: [ + fetchShardingKeysOptions.session, + countOptions.session, + findOptions.session, + ], outdated: false, error: null, count: null, // we don't know the new count yet @@ -1123,7 +1133,7 @@ const configureStore = (options = {}) => { Object.assign(stateChanges, { abortController: null, - session: null, + sessions: null, resultId: resultId(), }); @@ -1134,14 +1144,13 @@ const configureStore = (options = {}) => { }, async onAbort() { - const session = this.state.session; - if (!session) { + const { sessions } = this.state; + if (!sessions) { return; } - this.setState({ session: null }); - + this.setState({ sessions: null }); try { - await this.dataService.killSession(session); + await this.dataService.killSession(sessions); } catch (err) { log.warn(mongoLogId(1001000096), 'Documents', 'Attempting to kill the session failed'); } diff --git a/packages/compass-crud/src/stores/crud-store.spec.js b/packages/compass-crud/src/stores/crud-store.spec.js index c0ef6c3b176..ffce42a97dd 100644 --- a/packages/compass-crud/src/stores/crud-store.spec.js +++ b/packages/compass-crud/src/stores/crud-store.spec.js @@ -159,7 +159,7 @@ describe('store', function() { expect(store.state).to.deep.equal({ abortController: null, - session: null, + sessions: null, debouncingLoad: false, loadingCount: false, collection: '', @@ -1047,7 +1047,7 @@ describe('store', function() { expect(state.status).to.equal('fetching'); expect(state.abortController).to.not.be.null; - expect(state.session).to.not.be.null; + expect(state.sessions).to.not.be.null; expect(state.outdated).to.be.false; expect(state.error).to.be.null; }, @@ -1076,7 +1076,7 @@ describe('store', function() { expect(state.shardKeys).to.deep.equal({}); expect(state.abortController).to.be.null; - expect(state.session).to.be.null; + expect(state.sessions).to.be.null; expect(state.resultId).to.not.equal(resultId); } ]); @@ -1565,7 +1565,7 @@ describe('store', function() { }); }); - it('aborts the queries and kills the session', async() => { + it('aborts the queries and kills the sessions', async() => { const spy = sinon.spy(dataService, 'aggregate'); const listener = waitForStates(store, [ @@ -1576,7 +1576,7 @@ describe('store', function() { expect(state.loadingCount).to.be.true; // initially count is still loading expect(state.error).to.be.null; expect(state.abortController).to.not.be.null; - expect(state.session).to.not.be.null; + expect(state.sessions).to.not.be.null; store.cancelOperation(); }, @@ -1588,7 +1588,7 @@ describe('store', function() { (state) => { // onAbort cleans up state.session - expect(state.session).to.be.null; + expect(state.sessions).to.be.null; }, (state) => { @@ -1596,7 +1596,7 @@ describe('store', function() { expect(state.status).to.equal('error'); expect(state.error.message).to.equal('The operation was cancelled.'); expect(state.abortController).to.be.null; - expect(state.session).to.be.null; + expect(state.sessions).to.be.null; expect(state.loadingCount).to.be.false; // eventually count loads } ]); @@ -1691,11 +1691,11 @@ describe('store', function() { })); expect(store.state.abortController).to.be.null; - expect(store.state.session).to.be.null; + expect(store.state.sessions).to.be.null; const promise = store.getPage(1); expect(store.state.abortController).to.not.be.null; - expect(store.state.session).to.not.be.null; + expect(store.state.sessions).to.not.be.null; await promise; expect(store.state.error.message).to.equal('This is a fake error.'); @@ -1705,15 +1705,15 @@ describe('store', function() { it('allows the operation to be cancelled', async() => { expect(store.state.abortController).to.be.null; - expect(store.state.session).to.be.null; + expect(store.state.sessions).to.be.null; const promise = store.getPage(1); expect(store.state.abortController).to.not.be.null; - expect(store.state.session).to.not.be.null; + expect(store.state.sessions).to.not.be.null; store.cancelOperation(); expect(store.state.abortController).to.be.null; - expect(store.state.session).to.be.null; + expect(store.state.sessions).to.be.null; expect(store.state.error).to.be.null; await promise; diff --git a/packages/data-service/src/data-service.ts b/packages/data-service/src/data-service.ts index ff426c76fc9..4ab84b77e15 100644 --- a/packages/data-service/src/data-service.ts +++ b/packages/data-service/src/data-service.ts @@ -1420,9 +1420,11 @@ class DataService extends EventEmitter { * Kill a session and terminate all in progress operations. * @param clientSession - a ClientSession (can be created with startSession()) */ - killSession(session: ClientSession): Promise { + killSession(sessions: ClientSession | ClientSession[]): Promise { return this._initializedClient.db('admin').command({ - killSessions: [session.id], + killSessions: Array.isArray(sessions) + ? sessions.map((s) => s.id) + : [sessions.id], }); }