[SERVER-55781] Verify shard version matches before allowing createIndexes in sharding opObserver Created: 05/Apr/21  Updated: 29/Oct/23  Resolved: 20/May/21

Status: Closed
Project: Core Server
Component/s: Sharding
Affects Version/s: None
Fix Version/s: 5.0.0-rc0, 5.1.0-rc0

Type: Bug Priority: Major - P3
Reporter: Blake Oler Assignee: Marcos José Grillo Ramirez
Resolution: Fixed Votes: 0
Labels: sharding-wfbf-day
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: File bf_repro.js     File create_indexes.diff    
Issue Links:
Backports
Problem/Incident
causes SERVER-57180 concurrent_create_indexes_with_drop_a... Closed
Related
related to SERVER-55879 Accommodate implicit collection creat... Closed
is related to SERVER-56774 [Resharding] Clear filtering metadata... Closed
Backwards Compatibility: Fully Compatible
Operating System: ALL
Backport Requested:
v5.0
Sprint: Sharding EMEA 2021-05-17, Sharding EMEA 2021-05-31
Participants:
Linked BF Score: 197

 Description   

Scenario

  1. Sharded cluster. A sharded collection db.foo exists with UUID existingUuid and epoch existingOid.
  2. A createIndexes call is sent to the cluster.
  3. The first attempt at creating an index fails on the shard due to a transient error.
  4. Before the shard starts the second attempt, another thread on the same shard drops db.foo.
  5. On the second attempt, the createIndexes call will see that the collection doesn't exist, and will implicitly create db.foo, unsharded, with UUID generatedUuid.
  6. The createIndexes command will succeed, with the requested indexes on the newly created db.foo.

This is incorrect behavior – the createIndexes call should see that the collection was dropped somehow and should fail the createIndexes call permanently.

Proposed Solution

Place a call to csr->checkShardVersionOrThrow() in a new function OpObserverShardingImpl::shardObserveCreateIndexes. This call will see that the newly created collection does not have epoch existingOid (because the collection is now unsharded), and will fail the operation.

Open Questions

  1. Will failing the createIndexes call at the point of the opObserver roll back the implicit collection creation?


 Comments   
Comment by Vivian Ge (Inactive) [ 06/Oct/21 ]

Updating the fixversion since branching activities occurred yesterday. This ticket will be in rc0 when it’s been triggered. For more active release information, please keep an eye on #server-release. Thank you!

Comment by Githook User [ 20/May/21 ]

Author:

{'name': 'Marcos Jose Grillo Ramirez', 'email': 'marcos.grillo@mongodb.com', 'username': 'm4nti5'}

Message: SERVER-55781 Add shard version check to create indexes path

(cherry picked from commit 7feb0ddaae387ea1720a2bee53f1e57a756dd359)
Branch: v5.0
https://github.com/mongodb/mongo/commit/eba1f0b63f2ad3bb922e538a9e197bccc5ac334f

Comment by Githook User [ 20/May/21 ]

Author:

{'name': 'Marcos Jose Grillo Ramirez', 'email': 'marcos.grillo@mongodb.com', 'username': 'm4nti5'}

Message: SERVER-55781 Add shard version check to create indexes path
Branch: master
https://github.com/mongodb/mongo/commit/7feb0ddaae387ea1720a2bee53f1e57a756dd359

Comment by Marcos José Grillo Ramirez [ 03/May/21 ]

After talking to blake.oler I'm going to refine a bit the bf_repro.js to add a create collection and ensuring that the shard where the create indexes command failed with the transient error is not the primary shard, that way we can check at the end that the UUID of the collections are different among the shards.

Comment by Marcos José Grillo Ramirez [ 30/Apr/21 ]

To answer the open question, yes, the collection creation is rollbacked if we throw from the shard op observer. From doing some local tests and reading the code, the procedure to create indexes on an empty unsharded collection first creates the collection implicitly, and then create the indexes, but given the fact that this is being done under a WriteUnitOfWork if we never reach the commit then everything is rollbacked.

Comment by Max Hirschhorn [ 07/Apr/21 ]

// The check above will succeed, and we will also get an error with the check UUIDs hook that the
// still-remaining info on the config server has a mismatched UUID with what's been created on the
// shard.

Dropping the collection locally on the shard with assertDropCollection() is slightly different from what happens during resharding. This is because, in resharding, dropping the collection locally on the donor shard was accompanied by an update to the collection metadata to say that the donor shard no longer owns chunks for the collection being dropped.

ShardingTest#checkUUIDsConsistentAcrossCluster() fails in bf_repro.js because the collection metadata says the shard owns chunks for the collection but doesn't have the collection in its collection catalog. The user-facing drop command wouldn't encounter that issue because the config.collections entry for the collection being dropped would also have been removed (and thus the shard is consistent with the config server).

However, the createIndexes command is still implicitly creating a new collection in the collection catalog. This is undesirable because it would lead a future chunk migration into this shard to fail. These extraneous collections are not new in sharding (see SERVER-54231 as an example from movePrimary) but it perhaps wasn't realized that createIndexes is another way this can manifest.

diff --git a/bf_repro.js b/bf_repro.js
index 3fabed694c..b6d6dd431b 100644
--- a/bf_repro.js
+++ b/bf_repro.js
@@ -49,7 +49,7 @@ createIndexesThread.start();
 hangOnAbortFailpoint.wait();
 
 // Assert that we drop the original collection.
-assertDropCollection(st.rs0.getPrimary().getDB(kDbName), collName);
+st.s.getDB(kDbName).getCollection(collName).drop();
 
 // Wait on the second retry, hang on the implicit collection creation/index creation path to
 // assert that the createIndexes command now knows that the collection is not sharded.

Comment by Blake Oler [ 05/Apr/21 ]

Attached is the bf repro jsTest, along with the diff in create_indexes.cpp that simulates a StaleConfigException.

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