SERVER-80786 Differentiate buckets and user time-series collections for sharded delete targetingr7.0.2-rc0

10 files changed, 155 insertions, 41 deletions

diff --git a/jstests/sharding/timeseries_buckets_modification_with_id.js b/jstests/sharding/timeseries_buckets_modification_with_id.js
new file mode 100644
index 00000000000..59063e237f6
--- /dev/null
+++ b/jstests/sharding/timeseries_buckets_modification_with_id.js
@@ -0,0 +1,72 @@
+/**
+ * Tests deleteOne works correctly on time-series buckets collections.
+ */
+
+(function() {
+"use strict";
+
+const st = new ShardingTest({shards: 2, rs: {nodes: 2}});
+
+const mongos = st.s;
+const testDB = mongos.getDB(jsTestName());
+const collName = "ts";
+const bucketsCollName = "system.buckets." + collName;
+const timeFieldName = "time";
+const metaFieldName = "tag";
+const bucketDoc = {
+    "_id": ObjectId("64dd4adcac4fd7e3ebbd9af3"),
+    "control": {
+        "version": 1,
+        "min":
+            {"_id": ObjectId("64dd4ae9a2c44e75d1151285"), "time": ISODate("2023-08-16T22:17:00Z")},
+        "max": {
+            "_id": ObjectId("64dd4ae9a2c44e75d1151285"),
+            "time": ISODate("2023-08-16T22:17:13.749Z")
+        }
+    },
+    "meta": 1,
+    "data": {
+        "_id": {"0": ObjectId("64dd4ae9a2c44e75d1151285")},
+        "time": {"0": ISODate("2023-08-16T22:17:13.749Z")}
+    }
+};
+
+function runTest(cmd, validateFn) {
+    const coll = testDB.getCollection(collName);
+    const bucketsColl = testDB.getCollection(bucketsCollName);
+    coll.drop();
+    assert.commandWorked(testDB.createCollection(
+        coll.getName(),
+        {timeseries: {timeField: timeFieldName, metaField: metaFieldName, granularity: "hours"}}));
+
+    assert.commandWorked(testDB.adminCommand({enableSharding: testDB.getName()}));
+    assert.commandWorked(
+        testDB.adminCommand({shardCollection: coll.getFullName(), key: {[metaFieldName]: 1}}));
+
+    assert.commandWorked(
+        testDB.adminCommand({split: testDB[bucketsCollName].getFullName(), middle: {meta: 1}}));
+    assert.commandWorked(testDB.adminCommand({
+        moveChunk: testDB[bucketsCollName].getFullName(),
+        find: {meta: 1},
+        to: st.getOther(st.getPrimaryShard(testDB.getName())).shardName,
+        _waitForDelete: true
+    }));
+
+    // Tests the command works for the buckets collection.
+    assert.commandWorked(bucketsColl.insert(bucketDoc));
+    assert.commandWorked(testDB.runCommand(cmd));
+    validateFn(bucketsColl);
+}
+
+function removeValidateFn(coll) {
+    assert.eq(coll.count(), 0);
+}
+
+runTest({
+    delete: bucketsCollName,
+    deletes: [{q: {_id: ObjectId("64dd4adcac4fd7e3ebbd9af3")}, limit: 1}]
+},
+        removeValidateFn);
+
+st.stop();
+})();
diff --git a/src/mongo/s/collection_routing_info_targeter.cpp b/src/mongo/s/collection_routing_info_targeter.cpp
index 93f780de552..3e19c2001a6 100644
--- a/src/mongo/s/collection_routing_info_targeter.cpp
+++ b/src/mongo/s/collection_routing_info_targeter.cpp
@@ -593,18 +593,17 @@ std::vector<ShardEndpoint> CollectionRoutingInfoTargeter::targetDelete(
 
     // Regular single deletes must target a single shard or be exact-ID.
     // Time-series single deletes must target a single shard.
-    auto isShardedTimeseriesCollection = isShardedTimeSeriesBucketsNamespace();
     uassert(ErrorCodes::ShardKeyNotFound,
             fmt::format("A single delete on a sharded {} contain the shard key (and have the "
                         "simple collation). Delete request: {}, shard key pattern: {}",
-                        isShardedTimeseriesCollection
+                        _isRequestOnTimeseriesViewNamespace
                             ? "time-series collection must"
                             : "collection must contain an exact match on _id (and have the "
                               "collection default collation) or",
                         deleteOp.toBSON().toString(),
                         _cri.cm.getShardKeyPattern().toString()),
             !_cri.cm.isSharded() || deleteOp.getMulti() ||
-                (isExactIdQuery(opCtx, *cq, _cri.cm) && !isShardedTimeseriesCollection) ||
+                (isExactIdQuery(opCtx, *cq, _cri.cm) && !_isRequestOnTimeseriesViewNamespace) ||
                 feature_flags::gFeatureFlagUpdateOneWithoutShardKey.isEnabled(
                     serverGlobalParams.featureCompatibility));
 
@@ -758,6 +757,10 @@ bool CollectionRoutingInfoTargeter::isShardedTimeSeriesBucketsNamespace() const
     return _cri.cm.isSharded() && _cri.cm.getTimeseriesFields();
 }
 
+bool CollectionRoutingInfoTargeter::isRequestOnTimeseriesViewNamespace() const {
+    return _isRequestOnTimeseriesViewNamespace;
+}
+
 bool CollectionRoutingInfoTargeter::timeseriesNamespaceNeedsRewrite(
     const NamespaceString& nss) const {
     return isShardedTimeSeriesBucketsNamespace() && !nss.isTimeseriesBucketsCollection();
diff --git a/src/mongo/s/collection_routing_info_targeter.h b/src/mongo/s/collection_routing_info_targeter.h
index 8dc910063f6..0b325065f26 100644
--- a/src/mongo/s/collection_routing_info_targeter.h
+++ b/src/mongo/s/collection_routing_info_targeter.h
@@ -125,6 +125,8 @@ public:
 
     bool isShardedTimeSeriesBucketsNamespace() const override;
 
+    bool isRequestOnTimeseriesViewNamespace() const override;
+
     bool timeseriesNamespaceNeedsRewrite(const NamespaceString& nss) const;
 
     const CollectionRoutingInfo& getRoutingInfo() const;
diff --git a/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp b/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
index 57bfe7487ad..dfcb930a44b 100644
--- a/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
+++ b/src/mongo/s/commands/cluster_find_and_modify_cmd.cpp
@@ -519,7 +519,13 @@ bool FindAndModifyCmd::run(OperationContext* opCtx,
         const bool isUpsert = cmdObjForShard.getBoolField("upsert");
         const BSONObj collation = getCollation(cmdObjForShard);
         if (write_without_shard_key::useTwoPhaseProtocol(
-                opCtx, nss, false /* isUpdateOrDelete */, isUpsert, query, collation)) {
+                opCtx,
+                nss,
+                false /* isUpdateOrDelete */,
+                isUpsert,
+                false /* isRequestOnTimeseriesViewNamespace */,
+                query,
+                collation)) {
             auto allowShardKeyUpdatesWithoutFullShardKeyInQuery =
                 opCtx->isRetryableWrite() || opCtx->inMultiDocumentTransaction();
 
@@ -780,12 +786,14 @@ void FindAndModifyCmd::_handleWouldChangeOwningShardErrorRetryableWriteLegacy(
         // from the opCtx (which has been set previously in Strategy).
         documentShardKeyUpdateUtil::startTransactionForShardKeyUpdate(opCtx);
 
-        if (write_without_shard_key::useTwoPhaseProtocol(opCtx,
-                                                         nss,
-                                                         false /* isUpdateOrDelete */,
-                                                         cmdObj.getBoolField("upsert"),
-                                                         cmdObj.getObjectField("query"),
-                                                         getCollation(cmdObj))) {
+        if (write_without_shard_key::useTwoPhaseProtocol(
+                opCtx,
+                nss,
+                false /* isUpdateOrDelete */,
+                cmdObj.getBoolField("upsert"),
+                false /* isRequestOnTimeseriesViewNamespace */,
+                cmdObj.getObjectField("query"),
+                getCollation(cmdObj))) {
             _runCommandWithoutShardKey(opCtx,
                                        nss,
                                        stripWriteConcern(cmdObj),
diff --git a/src/mongo/s/mock_ns_targeter.h b/src/mongo/s/mock_ns_targeter.h
index 21cdd3d13a9..9fcb917384c 100644
--- a/src/mongo/s/mock_ns_targeter.h
+++ b/src/mongo/s/mock_ns_targeter.h
@@ -137,6 +137,10 @@ public:
         return false;
     }
 
+    bool isRequestOnTimeseriesViewNamespace() const override {
+        return false;
+    }
+
 private:
     /**
      * Returns the first ShardEndpoint for the query from the mock ranges. Only handles queries of
diff --git a/src/mongo/s/ns_targeter.h b/src/mongo/s/ns_targeter.h
index 5798438b639..737663d9146 100644
--- a/src/mongo/s/ns_targeter.h
+++ b/src/mongo/s/ns_targeter.h
@@ -72,6 +72,8 @@ public:
 
     virtual bool isShardedTimeSeriesBucketsNamespace() const = 0;
 
+    virtual bool isRequestOnTimeseriesViewNamespace() const = 0;
+
     /**
      * Returns a ShardEndpoint for a single document write or throws ShardKeyNotFound if 'doc' is
      * malformed with respect to the shard key pattern of the collection.
diff --git a/src/mongo/s/write_ops/batch_write_op.cpp b/src/mongo/s/write_ops/batch_write_op.cpp
index 549ca807e4b..61803911400 100644
--- a/src/mongo/s/write_ops/batch_write_op.cpp
+++ b/src/mongo/s/write_ops/batch_write_op.cpp
@@ -422,12 +422,14 @@ StatusWith<bool> targetWriteOps(OperationContext* opCtx,
             }
 
             if (!isMultiWrite &&
-                write_without_shard_key::useTwoPhaseProtocol(opCtx,
-                                                             targeter.getNS(),
-                                                             true /* isUpdateOrDelete */,
-                                                             isUpsert,
-                                                             query,
-                                                             collation)) {
+                write_without_shard_key::useTwoPhaseProtocol(
+                    opCtx,
+                    targeter.getNS(),
+                    true /* isUpdateOrDelete */,
+                    isUpsert,
+                    targeter.isRequestOnTimeseriesViewNamespace(),
+                    query,
+                    collation)) {
 
                 // Writes without shard key should be in their own batch.
                 if (!batchMap.empty()) {
diff --git a/src/mongo/s/write_ops/write_without_shard_key_util.cpp b/src/mongo/s/write_ops/write_without_shard_key_util.cpp
index dbd7d01fe56..6eb34a54533 100644
--- a/src/mongo/s/write_ops/write_without_shard_key_util.cpp
+++ b/src/mongo/s/write_ops/write_without_shard_key_util.cpp
@@ -176,6 +176,7 @@ bool useTwoPhaseProtocol(OperationContext* opCtx,
                          NamespaceString nss,
                          bool isUpdateOrDelete,
                          bool isUpsert,
+                         bool isRequestOnTimeseriesViewNamespace,
                          const BSONObj& query,
                          const BSONObj& collation) {
     if (!feature_flags::gFeatureFlagUpdateOneWithoutShardKey.isEnabled(
@@ -200,23 +201,21 @@ bool useTwoPhaseProtocol(OperationContext* opCtx,
     auto hasDefaultCollation =
         CollatorInterface::collatorsMatch(collator.get(), cm.getDefaultCollator());
 
-    auto tsFields = cm.getTimeseriesFields();
-    bool isTimeseries = tsFields.has_value();
-
     // updateOne and deleteOne do not use the two phase protocol for single writes that specify
     // _id in their queries, unless a document is being upserted. An exact _id match requires
     // default collation if the _id value is a collatable type.
     if (isUpdateOrDelete && query.hasField("_id") &&
         isExactIdQuery(opCtx, nss, query, collation, hasDefaultCollation) && !isUpsert &&
-        !isTimeseries) {
+        !isRequestOnTimeseriesViewNamespace) {
         return false;
     }
 
     BSONObj deleteQuery = query;
-    if (isTimeseries) {
+    if (isRequestOnTimeseriesViewNamespace) {
+        invariant(cm.getTimeseriesFields().has_value());
         auto expCtx = make_intrusive<ExpressionContext>(opCtx, std::move(collator), nss);
-        deleteQuery =
-            timeseries::getBucketLevelPredicateForRouting(query, expCtx, tsFields->getMetaField());
+        deleteQuery = timeseries::getBucketLevelPredicateForRouting(
+            query, expCtx, cm.getTimeseriesFields()->getMetaField());
     }
 
     auto shardKey = uassertStatusOK(
diff --git a/src/mongo/s/write_ops/write_without_shard_key_util.h b/src/mongo/s/write_ops/write_without_shard_key_util.h
index ed4ab44e859..b1ad28b40e4 100644
--- a/src/mongo/s/write_ops/write_without_shard_key_util.h
+++ b/src/mongo/s/write_ops/write_without_shard_key_util.h
@@ -51,6 +51,7 @@ bool useTwoPhaseProtocol(OperationContext* opCtx,
                          NamespaceString ns,
                          bool isUpdateOrDelete,
                          bool isUpsert,
+                         bool isRequestOnTimeseriesViewNamespace,
                          const BSONObj& query,
                          const BSONObj& collation);
 
diff --git a/src/mongo/s/write_ops/write_without_shard_key_util_test.cpp b/src/mongo/s/write_ops/write_without_shard_key_util_test.cpp
index 363fbc9e19b..6aff8464195 100644
--- a/src/mongo/s/write_ops/write_without_shard_key_util_test.cpp
+++ b/src/mongo/s/write_ops/write_without_shard_key_util_test.cpp
@@ -110,16 +110,19 @@ TEST_F(WriteWithoutShardKeyUtilTest, WriteQueryContainingFullShardKeyCanTargetSi
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("a" << 1 << "b" << 1),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, false);
 
-    useTwoPhaseProtocol = write_without_shard_key::useTwoPhaseProtocol(getOpCtx(),
-                                                                       kNss,
-                                                                       false /* isUpdateOrDelete */,
-                                                                       false /* isUpsert */,
-                                                                       BSON("a" << 1 << "b" << 1),
-                                                                       {} /* collation */);
+    useTwoPhaseProtocol =
+        write_without_shard_key::useTwoPhaseProtocol(getOpCtx(),
+                                                     kNss,
+                                                     false /* isUpdateOrDelete */,
+                                                     false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
+                                                     BSON("a" << 1 << "b" << 1),
+                                                     {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, false);
 }
 
@@ -132,16 +135,19 @@ TEST_F(WriteWithoutShardKeyUtilTest,
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("a" << 1),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, true);
 
-    useTwoPhaseProtocol = write_without_shard_key::useTwoPhaseProtocol(getOpCtx(),
-                                                                       kNss,
-                                                                       false /* isUpdateOrDelete */,
-                                                                       false /* isUpsert */,
-                                                                       BSON("a" << 1),
-                                                                       {} /* collation */);
+    useTwoPhaseProtocol =
+        write_without_shard_key::useTwoPhaseProtocol(getOpCtx(),
+                                                     kNss,
+                                                     false /* isUpdateOrDelete */,
+                                                     false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
+                                                     BSON("a" << 1),
+                                                     {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, true);
 }
 
@@ -154,6 +160,7 @@ TEST_F(WriteWithoutShardKeyUtilTest,
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("_id" << 1),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, false);
@@ -168,16 +175,19 @@ TEST_F(WriteWithoutShardKeyUtilTest,
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("x" << 1),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, true);
 
-    useTwoPhaseProtocol = write_without_shard_key::useTwoPhaseProtocol(getOpCtx(),
-                                                                       kNss,
-                                                                       false /* isUpdateOrDelete */,
-                                                                       false /* isUpsert */,
-                                                                       BSON("x" << 1),
-                                                                       {} /* collation */);
+    useTwoPhaseProtocol =
+        write_without_shard_key::useTwoPhaseProtocol(getOpCtx(),
+                                                     kNss,
+                                                     false /* isUpdateOrDelete */,
+                                                     false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
+                                                     BSON("x" << 1),
+                                                     {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, true);
 }
 
@@ -189,6 +199,7 @@ TEST_F(WriteWithoutShardKeyUtilTest, FindAndModifyQueryWithOnlyIdMustUseTwoPhase
                                                      kNss,
                                                      false /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("_id" << 1),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, true);
@@ -202,6 +213,7 @@ TEST_F(WriteWithoutShardKeyUtilTest, FindAndModifyQueryWithoutShardKeyMustUseTwo
                                                      kNss,
                                                      false /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("x" << 1),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, true);
@@ -215,6 +227,7 @@ TEST_F(WriteWithoutShardKeyUtilTest, QueryWithFeatureFlagDisabledDoesNotUseTwoPh
                                                      kNss,
                                                      false /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("x" << 1),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, false);
@@ -248,6 +261,7 @@ TEST_F(UnshardedCollectionTest, UnshardedCollectionDoesNotUseTwoPhaseProtocol) {
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("x" << 1),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, false);
@@ -262,6 +276,7 @@ TEST_F(WriteWithoutShardKeyUtilTest,
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("a"
                                                           << "a"
                                                           << "b"
@@ -280,6 +295,7 @@ TEST_F(WriteWithoutShardKeyUtilTest,
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("a" << 1 << "b" << 1),
                                                      BSON("collation"
                                                           << "lowercase") /* collation */);
@@ -295,6 +311,7 @@ TEST_F(WriteWithoutShardKeyUtilTest,
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("_id"
                                                           << "hello"),
                                                      BSON("collation"
@@ -311,6 +328,7 @@ TEST_F(WriteWithoutShardKeyUtilTest,
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("_id" << 1),
                                                      BSON("collation"
                                                           << "lowercase") /* collation */);
@@ -325,6 +343,7 @@ TEST_F(WriteWithoutShardKeyUtilTest, WriteQueryWithOnlyIdAndUpsertUsesTwoPhasePr
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      true /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("_id" << BSON("$eq" << 1)),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, true);
@@ -339,6 +358,7 @@ TEST_F(WriteWithoutShardKeyUtilTest,
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      true /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("a" << 1 << "_id" << BSON("$eq" << 1)),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, true);
@@ -353,6 +373,7 @@ TEST_F(WriteWithoutShardKeyUtilTest,
                                                      kNss,
                                                      true /* isUpdateOrDelete */,
                                                      false /* isUpsert */,
+                                                     false /* isRequestOnTimeseriesViewNamespace */,
                                                      BSON("_id" << BSON("$gt" << 1)),
                                                      {} /* collation */);
     ASSERT_EQ(useTwoPhaseProtocol, true);