diff --git a/src/mongo/db/s/resharding/recipient_document.idl b/src/mongo/db/s/resharding/recipient_document.idl index ba54c576a01..38ed97b748f 100644 --- a/src/mongo/db/s/resharding/recipient_document.idl +++ b/src/mongo/db/s/resharding/recipient_document.idl @@ -93,8 +93,7 @@ structs: optional: true description: "The timestamp for the snapshot read while cloning from the donors." skipCloningAndApplying: - type: bool - optional: true + type: optionalBool description: "Whether this recipient can skip cloning documents and fetching/applying oplog entries because it is not going to own any chunks for the collection after resharding." diff --git a/src/mongo/db/s/resharding/resharding_donor_recipient_common.cpp b/src/mongo/db/s/resharding/resharding_donor_recipient_common.cpp index 89d512d9992..3fa553594bf 100644 --- a/src/mongo/db/s/resharding/resharding_donor_recipient_common.cpp +++ b/src/mongo/db/s/resharding/resharding_donor_recipient_common.cpp @@ -374,15 +374,16 @@ ReshardingRecipientDocument constructRecipientDocumentFromReshardingFields( recipientDoc.setMetrics(std::move(metrics)); recipientDoc.setCommonReshardingMetadata(std::move(commonMetadata)); - const bool skipCloningAndApplying = - resharding::gFeatureFlagReshardingSkipCloningAndApplyingIfApplicable.isEnabled( - serverGlobalParams.featureCompatibility.acquireFCVSnapshot()) && - !metadata.currentShardHasAnyChunks(); - recipientDoc.setSkipCloningAndApplying(skipCloningAndApplying); - recipientDoc.setStoreOplogFetcherProgress( - resharding::gFeatureFlagReshardingStoreOplogFetcherProgress.isEnabled( - serverGlobalParams.featureCompatibility.acquireFCVSnapshot())); + if (resharding::gFeatureFlagReshardingSkipCloningAndApplyingIfApplicable.isEnabled( + serverGlobalParams.featureCompatibility.acquireFCVSnapshot()) && + !metadata.currentShardHasAnyChunks()) { + recipientDoc.setSkipCloningAndApplying(true); + } + if (resharding::gFeatureFlagReshardingStoreOplogFetcherProgress.isEnabled( + serverGlobalParams.featureCompatibility.acquireFCVSnapshot())) { + recipientDoc.setStoreOplogFetcherProgress(true); + } recipientDoc.setOplogBatchTaskCount(recipientFields->getOplogBatchTaskCount()); diff --git a/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.cpp b/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.cpp index 12dee7aa54c..d85926c3453 100644 --- a/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.cpp +++ b/src/mongo/db/s/resharding/resharding_donor_recipient_common_test.cpp @@ -354,8 +354,12 @@ private: struct DonorFieldsValidator { void validate(ReshardingDonorDocument doc) { + // 'performVerification' should only be set if it is specified. if (performVerification) { ASSERT_EQ(doc.getPerformVerification(), *performVerification); + } else { + ASSERT_FALSE(doc.getPerformVerification().has_value()); + ASSERT_EQ(doc.getPerformVerification(), false); } } @@ -364,19 +368,31 @@ struct DonorFieldsValidator { struct RecipientFieldsValidator { void validate(ReshardingRecipientDocument doc) { + // 'skipCloningAndApplying' should only be set if it is true. if (skipCloningAndApplying) { - ASSERT_EQ(doc.getSkipCloningAndApplying(), *skipCloningAndApplying); + ASSERT_EQ(doc.getSkipCloningAndApplying(), skipCloningAndApplying); + } else { + ASSERT_FALSE(doc.getSkipCloningAndApplying().has_value()); + ASSERT_EQ(doc.getSkipCloningAndApplying(), false); } + // 'storeOplogFetcherProgress' should only be set if it is true. if (storeOplogFetcherProgress) { - ASSERT_EQ(doc.getStoreOplogFetcherProgress(), *storeOplogFetcherProgress); + ASSERT_EQ(doc.getStoreOplogFetcherProgress(), storeOplogFetcherProgress); + } else { + ASSERT_FALSE(doc.getStoreOplogFetcherProgress().has_value()); + ASSERT_EQ(doc.getStoreOplogFetcherProgress(), false); } + // 'performVerification' should only be set if it is specified. if (performVerification) { ASSERT_EQ(doc.getPerformVerification(), *performVerification); + } else { + ASSERT_FALSE(doc.getPerformVerification().has_value()); + ASSERT_EQ(doc.getPerformVerification(), false); } } - boost::optional skipCloningAndApplying; - boost::optional storeOplogFetcherProgress; + bool skipCloningAndApplying; + bool storeOplogFetcherProgress; boost::optional performVerification; }; @@ -834,17 +850,35 @@ TEST_F(ReshardingDonorRecipientCommonTest, false /* expectDonorStateMachine */); } -TEST_F(ReshardingDonorRecipientCommonTest, ProcessDonorFieldsPerformVerificationUnspecified) { +TEST_F(ReshardingDonorRecipientCommonTest, + ProcessDonorFieldsPerformVerificationUnspecified_FeatureFlagEnabled) { + RAIIServerParameterControllerForTest verificationFeatureFlagController( + "featureFlagReshardingVerification", true); auto performVerification = boost::none; + testProcessDonorFields(kThisShard.getShardId() /* shardThatChunkExistsOn*/, kOtherShard.getShardId() /* primaryShard */, performVerification, true /* expectDonorStateMachine */, - DonorFieldsValidator{.performVerification = false}); + DonorFieldsValidator{}); +} + +TEST_F(ReshardingDonorRecipientCommonTest, + ProcessDonorFieldsPerformVerificationUnspecified_FeatureFlagDisabled) { + RAIIServerParameterControllerForTest verificationFeatureFlagController( + "featureFlagReshardingVerification", false); + auto performVerification = boost::none; + + testProcessDonorFields(kThisShard.getShardId() /* shardThatChunkExistsOn*/, + kOtherShard.getShardId() /* primaryShard */, + performVerification, + true /* expectDonorStateMachine */, + DonorFieldsValidator{}); } TEST_F(ReshardingDonorRecipientCommonTest, ProcessDonorFieldsNotPerformVerification) { bool performVerification = false; + testProcessDonorFields(kThisShard.getShardId() /* shardThatChunkExistsOn*/, kOtherShard.getShardId() /* primaryShard */, performVerification, @@ -856,8 +890,8 @@ TEST_F(ReshardingDonorRecipientCommonTest, ProcessDonorFieldsPerformVerification_FeatureFlagEnabled) { RAIIServerParameterControllerForTest verificationFeatureFlagController( "featureFlagReshardingVerification", true); - bool performVerification = true; + testProcessDonorFields(kThisShard.getShardId() /* shardThatChunkExistsOn*/, kOtherShard.getShardId() /* primaryShard */, performVerification, @@ -869,8 +903,8 @@ TEST_F(ReshardingDonorRecipientCommonTest, ProcessDonorFieldsPerformVerification_FeatureFlagDisabled) { RAIIServerParameterControllerForTest verificationFeatureFlagController( "featureFlagReshardingVerification", false); - bool performVerification = true; + ASSERT_THROWS_CODE(testProcessDonorFields(kThisShard.getShardId() /* shardThatChunkExistsOn*/, kOtherShard.getShardId() /* primaryShard */, performVerification, @@ -935,17 +969,35 @@ TEST_F(ReshardingDonorRecipientCommonTest, RecipientFieldsValidator{.skipCloningAndApplying = false}); } -TEST_F(ReshardingDonorRecipientCommonTest, ProcessRecipientFieldsPerformVerificationUnspecified) { - auto performVerification = boost::none; +TEST_F(ReshardingDonorRecipientCommonTest, + ProcessRecipientFieldsPerformVerificationUnspecified_FeatureFlagEnabled) { + RAIIServerParameterControllerForTest verificationFeatureFlagController( + "featureFlagReshardingVerification", true); + boost::optional performVerification = boost::none; + testProcessRecipientFields(kThisShard.getShardId() /* shardThatChunkExistsOn*/, kOtherShard.getShardId() /* primaryShard */, performVerification, true /* expectRecipientStateMachine */, - RecipientFieldsValidator{.performVerification = false}); + RecipientFieldsValidator{}); +} + +TEST_F(ReshardingDonorRecipientCommonTest, + ProcessRecipientFieldsPerformVerificationUnspecified_FeatureFlagDisabled) { + RAIIServerParameterControllerForTest verificationFeatureFlagController( + "featureFlagReshardingVerification", false); + auto performVerification = boost::none; + + testProcessRecipientFields(kThisShard.getShardId() /* shardThatChunkExistsOn*/, + kOtherShard.getShardId() /* primaryShard */, + performVerification, + true /* expectRecipientStateMachine */, + RecipientFieldsValidator{}); } TEST_F(ReshardingDonorRecipientCommonTest, ProcessRecipientFieldsNotPerformVerification) { bool performVerification = false; + testProcessRecipientFields( kThisShard.getShardId() /* shardThatChunkExistsOn*/, kOtherShard.getShardId() /* primaryShard */, diff --git a/src/mongo/db/s/resharding/resharding_util.cpp b/src/mongo/db/s/resharding/resharding_util.cpp index 3872bfca625..03edbfdf4f9 100644 --- a/src/mongo/db/s/resharding/resharding_util.cpp +++ b/src/mongo/db/s/resharding/resharding_util.cpp @@ -554,7 +554,7 @@ void validatePerformVerification(boost::optional performVerification) { void validatePerformVerification(bool performVerification) { uassert(ErrorCodes::InvalidOptions, - str::stream() << "Cannot specify '" + str::stream() << "Cannot set '" << CommonReshardingMetadata::kPerformVerificationFieldName << "' to true when featureFlagReshardingVerification is not enabled", !performVerification ||