fix(compass-crud): Use separate sessions for all operations in compass-crud COMPASS-5299

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');
       }

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,15 +1588,15 @@ describe('store', function() {
 
           (state) => {
             // onAbort cleans up state.session
-            expect(state.session).to.be.null;
+            expect(state.sessions).to.be.null;
           },
 
           (state) => {
             // the operation should fail
             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;

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<Document> {
+  killSession(sessions: ClientSession | ClientSession[]): Promise<Document> {
     return this._initializedClient.db('admin').command({
-      killSessions: [session.id],
+      killSessions: Array.isArray(sessions)
+        ? sessions.map((s) => s.id)
+        : [sessions.id],
     });
   }