[SERVER-46651] Concurrent update with upsert:true and dropCollection can error Created: 05/Mar/20  Updated: 06/Dec/22

Status: Backlog
Project: Core Server
Component/s: Write Ops
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Maria van Keulen Assignee: Backlog - Query Execution
Resolution: Unresolved Votes: 0
Labels: qexec-team
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to SERVER-44409 Add FSM test for creating indexes/col... Closed
Assigned Teams:
Query Execution
Operating System: ALL
Steps To Reproduce:

jstests/concurrency/fsm_workloads/CRUD_and_commands.js
diff --git a/jstests/concurrency/fsm_workloads/CRUD_and_commands.js b/jstests/concurrency/fsm_workloads/CRUD_and_commands.js
new file mode 100644
index 0000000000..6fc472dd93
--- /dev/null
+++ b/jstests/concurrency/fsm_workloads/CRUD_and_commands.js
@@ -0,0 +1,223 @@
+'use strict';
+
+/**
+ * Perform CRUD operations, some of which may implicitly create collections. Also perform index
+ * creations which may implicitly create collections.
+ */
+
+var $config = (function() {
+    const data = {numIds: 10, numDocsToInsertPerThread: 5, valueToBeInserted: 1, batchSize: 50};
+
+    const states = {
+        init: function init(db, collName) {
+            this.session = db.getMongo().startSession({causalConsistency: true});
+            this.sessionDb = this.session.getDatabase(db.getName());
+            this.docValue = "mydoc";
+        },
+
+        insertDocs: function insertDocs(db, collName) {
+            for (let i = 0; i < 5; i++) {
+                const res = db[collName].insert({value: this.docValue, num: 1});
+                assertWhenOwnColl.commandWorked(res);
+                assertWhenOwnColl.eq(1, res.nInserted);
+            }
+        },
+
+        updateDocs: function updateDocs(db, collName) {
+            for (let i = 0; i < 5; ++i) {
+                let indexToUpdate = Math.floor(Math.random() * this.numIds);
+                try {
+                    assertWhenOwnColl.commandWorked(db[collName].update(
+                        {_id: indexToUpdate}, {$inc: {num: 1}}, {upsert: true}));
+                } catch (e) {
+                    // We propagate TransientTransactionErrors to allow the state function to
+                    // automatically be retried when TestData.runInsideTransaction=true
+                    if (e.hasOwnProperty('errorLabels') &&
+                        e.errorLabels.includes('TransientTransactionError')) {
+                        throw e;
+                    }
+                    // dropIndex can cause queries to throw if these queries yield.
+                    assertAlways.contains(e.code,
+                                          [ErrorCodes.QueryPlanKilled, ErrorCodes.OperationFailed],
+                                          'unexpected error code: ' + e.code + ': ' + e.message);
+                }
+            }
+        },
+
+        readDocs: function readDocs(db, collName) {
+            for (let i = 0; i < 5; ++i) {
+                try {
+                    let res = db[collName].findOne({value: this.docValue});
+                    if (res !== null) {
+                        assertAlways.eq(this.docValue, res.value);
+                    }
+                } catch (e) {
+                    // We propagate TransientTransactionErrors to allow the state function to
+                    // automatically be retried when TestData.runInsideTransaction=true
+                    if (e.hasOwnProperty('errorLabels') &&
+                        e.errorLabels.includes('TransientTransactionError')) {
+                        throw e;
+                    }
+                    // dropIndex can cause queries to throw if these queries yield.
+                    assertAlways.contains(e.code,
+                                          [ErrorCodes.QueryPlanKilled, ErrorCodes.OperationFailed],
+                                          'unexpected error code: ' + e.code + ': ' + e.message);
+                }
+            }
+        },
+
+        deleteDocs: function deleteDocs(db, collName) {
+            let indexToDelete = Math.floor(Math.random() * this.numIds);
+            try {
+                db[collName].deleteOne({_id: indexToDelete});
+            } catch (e) {
+                // We propagate TransientTransactionErrors to allow the state function to
+                // automatically be retried when TestData.runInsideTransaction=true
+                if (e.hasOwnProperty('errorLabels') &&
+                    e.errorLabels.includes('TransientTransactionError')) {
+                    throw e;
+                }
+                // dropIndex can cause queries to throw if these queries yield.
+                assertAlways.contains(e.code,
+                                      [ErrorCodes.QueryPlanKilled, ErrorCodes.OperationFailed],
+                                      'unexpected error code: ' + e.code + ': ' + e.message);
+            }
+        },
+
+        createIndex: function createIndex(db, collName) {
+            db[collName].createIndex({value: 1});
+        },
+
+        createIdIndex: function createIdIndex(db, collName) {
+            assertWhenOwnColl.commandWorked(db[collName].createIndex({_id: 1}));
+        },
+
+        dropIndex: function dropIndex(db, collName) {
+            db[collName].dropIndex({value: 1});
+        },
+
+        dropCollection: function dropCollection(db, collName) {
+            db[collName].drop();
+            /*
+            // The above drop forces this state to be run outside of a transaction, so it is fine to
+            // ignore DuplicateKey errors here.
+            for (let i = 0; i < this.numIds; i++) {
+                const res = db[collName].insert({_id: i, value: this.docValue, num: 1});
+                assertAlways.commandWorkedOrFailedWithCode(res, ErrorCodes.DuplicateKey);
+            }*/
+        }
+
+    };
+
+    const transitions = {
+        init: {
+            insertDocs: 0.10,
+            updateDocs: 0.10,
+            readDocs: 0.10,
+            deleteDocs: 0.10,
+            createIndex: 0.10,
+            createIdIndex: 0.10,
+            dropIndex: 0.10,
+            dropCollection: 0.10
+        },
+        insertDocs: {
+            insertDocs: 0.10,
+            updateDocs: 0.10,
+            readDocs: 0.10,
+            deleteDocs: 0.10,
+            createIndex: 0.10,
+            createIdIndex: 0.10,
+            dropIndex: 0.10,
+            dropCollection: 0.30
+        },
+        updateDocs: {
+            insertDocs: 0.10,
+            updateDocs: 0.10,
+            readDocs: 0.10,
+            deleteDocs: 0.10,
+            createIndex: 0.10,
+            createIdIndex: 0.10,
+            dropIndex: 0.10,
+            dropCollection: 0.30
+        },
+        readDocs: {
+            insertDocs: 0.10,
+            updateDocs: 0.10,
+            readDocs: 0.10,
+            deleteDocs: 0.10,
+            createIndex: 0.10,
+            createIdIndex: 0.10,
+            dropIndex: 0.10,
+            dropCollection: 0.30
+        },
+        deleteDocs: {
+            insertDocs: 0.10,
+            updateDocs: 0.10,
+            readDocs: 0.10,
+            deleteDocs: 0.10,
+            createIndex: 0.10,
+            createIdIndex: 0.10,
+            dropIndex: 0.10,
+            dropCollection: 0.30
+        },
+        createIndex: {
+            insertDocs: 0.10,
+            updateDocs: 0.10,
+            readDocs: 0.10,
+            deleteDocs: 0.10,
+            createIndex: 0.10,
+            createIdIndex: 0.10,
+            dropIndex: 0.10,
+            dropCollection: 0.30
+        },
+        createIdIndex: {
+            insertDocs: 0.10,
+            updateDocs: 0.10,
+            readDocs: 0.10,
+            deleteDocs: 0.10,
+            createIndex: 0.10,
+            createIdIndex: 0.10,
+            dropIndex: 0.10,
+            dropCollection: 0.30
+        },
+        dropIndex: {
+            insertDocs: 0.10,
+            updateDocs: 0.10,
+            readDocs: 0.10,
+            deleteDocs: 0.10,
+            createIndex: 0.10,
+            createIdIndex: 0.10,
+            dropIndex: 0.10,
+            dropCollection: 0.30
+        },
+        dropCollection: {
+            insertDocs: 0.10,
+            updateDocs: 0.10,
+            readDocs: 0.10,
+            deleteDocs: 0.10,
+            createIndex: 0.10,
+            createIdIndex: 0.10,
+            dropIndex: 0.10,
+            dropCollection: 0.10
+        }
+    };
+
+    function setup(db, collName, cluster) {
+        assertAlways.commandWorked(db.runCommand({create: collName}));
+        for (let i = 0; i < this.numIds; i++) {
+            const res = db[collName].insert({_id: i, value: this.docValue, num: 1});
+            assertAlways.commandWorked(res);
+            assert.eq(1, res.nInserted);
+        }
+    }
+
+    return {
+        threadCount: 5,
+        iterations: 10,
+        startState: 'init',
+        states: states,
+        transitions: transitions,
+        setup: setup,
+        data: data,
+    };
+})();

Participants:

 Description   

I am adding a concurrency workload (included in steps to reproduce) as part of SERVER-44409 that, among other operations, performs updates with upsert:true concurrently with dropCollection. This workload can fail with the following error:
Exec error resulting in state FAILURE :: caused by :: collection dropped. UUID dbaf99a7-24f5-4054-ac60-116b0f97a283
I did not observe this error when the workload was run as part of our transactions override concurrency testing, which leads me to believe there is some window of time when locks do not conflict and somehow a collection can be dropped before the update occurs.

I would expect these two operations to safely execute concurrently, since update should just create the collection if upsert is true.



 Comments   
Comment by Maria van Keulen [ 05/Mar/20 ]

This comment in another FSM test suggests that update may be failing due to queries yielding and the dropCollection coming in after the yield. I've confirmed with jacob.evans that this scenario makes sense, so passing to Query to investigate how to address this.

Generated at Thu Feb 08 05:12:03 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.