diff options
author | David Storch <david.storch@10gen.com> | 2018-11-07 11:04:09 -0500 |
---|---|---|
committer | David Storch <david.storch@10gen.com> | 2018-11-12 10:29:15 -0500 |
commit | 7369fd49c9d0c348406e08a3308d6e12cdcb057a (patch) | |
tree | 4941508e4f61ea73411699161ab25f8bcb129582 | |
parent | bc8bfc6b8ad5ebf05090ae49f8fa8bf35d028d28 (diff) | |
download | mongo-7369fd49c9d0c348406e08a3308d6e12cdcb057a.tar.gz |
SERVER-37446 Make UPDATE and DELETE inherit from RequiresMutableCollectionStage.
Also deletes UpdateLifecyle, which was used as part of the
UpdateStage's yield recovery, but is no longer necessary.
27 files changed, 264 insertions, 464 deletions
diff --git a/jstests/core/explain_delete.js b/jstests/core/explain_delete.js index 55462fc2663..9599c7df9b8 100644 --- a/jstests/core/explain_delete.js +++ b/jstests/core/explain_delete.js @@ -1,64 +1,67 @@ // @tags: [requires_non_retryable_writes, requires_fastcount] // Tests for explaining the delete command. +(function() { + "use strict"; -var collName = "jstests_explain_delete"; -var t = db[collName]; -t.drop(); + var collName = "jstests_explain_delete"; + var t = db[collName]; + t.drop(); -var explain; + var explain; -/** - * Verify that the explain command output 'explain' shows a DELETE stage with an nWouldDelete - * value equal to 'nWouldDelete'. - */ -function checkNWouldDelete(explain, nWouldDelete) { - printjson(explain); - assert.commandWorked(explain); - assert("executionStats" in explain); - var executionStats = explain.executionStats; - assert("executionStages" in executionStats); + /** + * Verify that the explain command output 'explain' shows a DELETE stage with an nWouldDelete + * value equal to 'nWouldDelete'. + */ + function checkNWouldDelete(explain, nWouldDelete) { + assert.commandWorked(explain); + assert("executionStats" in explain); + var executionStats = explain.executionStats; + assert("executionStages" in executionStats); - // If passed through mongos, then DELETE stage(s) should be below the SHARD_WRITE mongos stage. - // Otherwise the DELETE stage is the root stage. - var execStages = executionStats.executionStages; - if ("SHARD_WRITE" === execStages.stage) { - let totalToBeDeletedAcrossAllShards = 0; - execStages.shards.forEach(function(shardExplain) { - const rootStageName = shardExplain.executionStages.stage; - assert.eq(rootStageName, "DELETE", tojson(execStages)); - totalToBeDeletedAcrossAllShards += shardExplain.executionStages.nWouldDelete; - }); - assert.eq(totalToBeDeletedAcrossAllShards, nWouldDelete); - } else { - assert.eq(execStages.stage, "DELETE"); - assert.eq(execStages.nWouldDelete, nWouldDelete); + // If passed through mongos, then DELETE stage(s) should be below the SHARD_WRITE mongos + // stage. Otherwise the DELETE stage is the root stage. + var execStages = executionStats.executionStages; + if ("SHARD_WRITE" === execStages.stage) { + let totalToBeDeletedAcrossAllShards = 0; + execStages.shards.forEach(function(shardExplain) { + const rootStageName = shardExplain.executionStages.stage; + assert.eq(rootStageName, "DELETE", tojson(execStages)); + totalToBeDeletedAcrossAllShards += shardExplain.executionStages.nWouldDelete; + }); + assert.eq(totalToBeDeletedAcrossAllShards, nWouldDelete, explain); + } else { + assert.eq(execStages.stage, "DELETE", explain); + assert.eq(execStages.nWouldDelete, nWouldDelete, explain); + } } -} -// Explain delete against an empty collection. -explain = db.runCommand({explain: {delete: collName, deletes: [{q: {a: 1}, limit: 0}]}}); -checkNWouldDelete(explain, 0); + // Explain delete against an empty collection. + assert.commandWorked(db.createCollection(t.getName())); + explain = db.runCommand({explain: {delete: collName, deletes: [{q: {a: 1}, limit: 0}]}}); + checkNWouldDelete(explain, 0); -// Add an index but no data, and check that the explain still works. -t.ensureIndex({a: 1}); -explain = db.runCommand({explain: {delete: collName, deletes: [{q: {a: 1}, limit: 0}]}}); -checkNWouldDelete(explain, 0); + // Add an index but no data, and check that the explain still works. + t.ensureIndex({a: 1}); + explain = db.runCommand({explain: {delete: collName, deletes: [{q: {a: 1}, limit: 0}]}}); + checkNWouldDelete(explain, 0); -// Add some copies of the same document. -for (var i = 0; i < 10; i++) { - t.insert({a: 1}); -} -assert.eq(10, t.count()); + // Add some copies of the same document. + for (var i = 0; i < 10; i++) { + t.insert({a: 1}); + } + assert.eq(10, t.count()); -// Run an explain which shows that all 10 documents *would* be deleted. -explain = db.runCommand({explain: {delete: collName, deletes: [{q: {a: 1}, limit: 0}]}}); -checkNWouldDelete(explain, 10); + // Run an explain which shows that all 10 documents *would* be deleted. + explain = db.runCommand({explain: {delete: collName, deletes: [{q: {a: 1}, limit: 0}]}}); + checkNWouldDelete(explain, 10); -// Make sure all 10 documents are still there. -assert.eq(10, t.count()); + // Make sure all 10 documents are still there. + assert.eq(10, t.count()); -// If we run the same thing without the explain, then all 10 docs should be deleted. -var deleteResult = db.runCommand({delete: collName, deletes: [{q: {a: 1}, limit: 0}]}); -assert.commandWorked(deleteResult); -assert.eq(0, t.count()); + // If we run the same thing without the explain, then all 10 docs should be deleted. + var deleteResult = db.runCommand({delete: collName, deletes: [{q: {a: 1}, limit: 0}]}); + assert.commandWorked(deleteResult); + assert.eq(0, t.count()); +}()); diff --git a/jstests/core/explain_writecmd_nonexistent_collection.js b/jstests/core/explain_writecmd_nonexistent_collection.js new file mode 100644 index 00000000000..2d3080357b5 --- /dev/null +++ b/jstests/core/explain_writecmd_nonexistent_collection.js @@ -0,0 +1,40 @@ +// Test explaining a delete command against a non-existent collection. +// +// @tags: [requires_non_retryable_writes, requires_fastcount, +// assumes_no_implicit_collection_creation_after_drop] +(function() { + "use strict"; + + load("jstests/libs/analyze_plan.js"); + + function assertCollectionDoesNotExist(collName) { + const collectionList = db.getCollectionInfos({name: collName}); + assert.eq(0, collectionList.length, collectionList); + } + + const collName = "explain_delete_nonexistent_collection"; + const coll = db[collName]; + coll.drop(); + + // Explain of delete against a non-existent collection returns an EOF plan. + let explain = assert.commandWorked( + db.runCommand({explain: {delete: collName, deletes: [{q: {a: 1}, limit: 0}]}})); + assert(planHasStage(db, explain.queryPlanner.winningPlan, "EOF"), explain); + assert(!planHasStage(db, explain.queryPlanner.winningPlan, "DELETE"), explain); + + assertCollectionDoesNotExist(collName); + + // Explain of an update with upsert:false returns an EOF plan. + explain = assert.commandWorked(db.runCommand( + {explain: {update: collName, updates: [{q: {a: 1}, u: {$set: {b: 1}}, upsert: false}]}})); + assert(planHasStage(db, explain.queryPlanner.winningPlan, "EOF"), explain); + assert(!planHasStage(db, explain.queryPlanner.winningPlan, "UPDATE"), explain); + assertCollectionDoesNotExist(collName); + + // Explain of an update with upsert:true returns an EOF plan, and does not create a collection. + explain = assert.commandWorked(db.runCommand( + {explain: {update: collName, updates: [{q: {a: 1}, u: {$set: {b: 1}}, upsert: true}]}})); + assert(planHasStage(db, explain.queryPlanner.winningPlan, "EOF"), explain); + assert(!planHasStage(db, explain.queryPlanner.winningPlan, "UPDATE"), explain); + assertCollectionDoesNotExist(collName); +}()); diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index ad054e77f68..599cd7912cd 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -1069,13 +1069,12 @@ env.Library( 'exec/write_stage_common.cpp', 'ops/parsed_delete.cpp', 'ops/parsed_update.cpp', - 'ops/update_lifecycle_impl.cpp', 'ops/update_result.cpp', - 'query/explain.cpp', - 'query/find.cpp', 'pipeline/document_source_cursor.cpp', 'pipeline/document_source_geo_near_cursor.cpp', 'pipeline/pipeline_d.cpp', + 'query/explain.cpp', + 'query/find.cpp', 'query/get_executor.cpp', 'query/internal_plans.cpp', 'query/plan_executor_impl.cpp', diff --git a/src/mongo/db/commands/find_and_modify.cpp b/src/mongo/db/commands/find_and_modify.cpp index e13aab6702b..0029ed9e9d5 100644 --- a/src/mongo/db/commands/find_and_modify.cpp +++ b/src/mongo/db/commands/find_and_modify.cpp @@ -44,6 +44,7 @@ #include "mongo/db/commands/find_and_modify_common.h" #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/db_raii.h" +#include "mongo/db/exec/delete.h" #include "mongo/db/exec/update.h" #include "mongo/db/exec/working_set_common.h" #include "mongo/db/lasterror.h" @@ -54,7 +55,6 @@ #include "mongo/db/ops/insert.h" #include "mongo/db/ops/parsed_delete.h" #include "mongo/db/ops/parsed_update.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/ops/update_request.h" #include "mongo/db/ops/write_ops_retryability.h" #include "mongo/db/query/explain.h" @@ -76,32 +76,6 @@ namespace mongo { namespace { -const UpdateStats* getUpdateStats(const PlanExecutor* exec) { - // The stats may refer to an update stage, or a projection stage wrapping an update stage. - if (StageType::STAGE_PROJECTION == exec->getRootStage()->stageType()) { - invariant(exec->getRootStage()->getChildren().size() == 1U); - invariant(StageType::STAGE_UPDATE == exec->getRootStage()->child()->stageType()); - const SpecificStats* stats = exec->getRootStage()->child()->getSpecificStats(); - return static_cast<const UpdateStats*>(stats); - } else { - invariant(StageType::STAGE_UPDATE == exec->getRootStage()->stageType()); - return static_cast<const UpdateStats*>(exec->getRootStage()->getSpecificStats()); - } -} - -const DeleteStats* getDeleteStats(const PlanExecutor* exec) { - // The stats may refer to a delete stage, or a projection stage wrapping a delete stage. - if (StageType::STAGE_PROJECTION == exec->getRootStage()->stageType()) { - invariant(exec->getRootStage()->getChildren().size() == 1U); - invariant(StageType::STAGE_DELETE == exec->getRootStage()->child()->stageType()); - const SpecificStats* stats = exec->getRootStage()->child()->getSpecificStats(); - return static_cast<const DeleteStats*>(stats); - } else { - invariant(StageType::STAGE_DELETE == exec->getRootStage()->stageType()); - return static_cast<const DeleteStats*>(exec->getRootStage()->getSpecificStats()); - } -} - /** * If the operation succeeded, then Status::OK() is returned, possibly with a document value * to return to the client. If no matching document to update or remove was found, then none @@ -135,7 +109,6 @@ boost::optional<BSONObj> advanceExecutor(OperationContext* opCtx, void makeUpdateRequest(const OperationContext* opCtx, const FindAndModifyRequest& args, bool explain, - UpdateLifecycleImpl* updateLifecycle, UpdateRequest* requestOut) { requestOut->setQuery(args.getQuery()); requestOut->setProj(args.getFields()); @@ -148,7 +121,6 @@ void makeUpdateRequest(const OperationContext* opCtx, : UpdateRequest::RETURN_OLD); requestOut->setMulti(false); requestOut->setExplain(explain); - requestOut->setLifecycle(updateLifecycle); const auto& readConcernArgs = repl::ReadConcernArgs::get(opCtx); requestOut->setYieldPolicy(readConcernArgs.getLevel() == @@ -181,9 +153,9 @@ void appendCommandResponse(const PlanExecutor* exec, const boost::optional<BSONObj>& value, BSONObjBuilder* result) { if (isRemove) { - find_and_modify::serializeRemove(getDeleteStats(exec)->docsDeleted, value, result); + find_and_modify::serializeRemove(DeleteStage::getNumDeleted(*exec), value, result); } else { - const auto updateStats = getUpdateStats(exec); + const auto updateStats = UpdateStage::getUpdateStats(exec); // Note we have to use the objInserted from the stats here, rather than 'value' because the // _id field could have been excluded by a projection. @@ -295,9 +267,8 @@ public: Explain::explainStages(exec.get(), collection, verbosity, &bodyBuilder); } else { UpdateRequest request(nsString); - UpdateLifecycleImpl updateLifecycle(nsString); const bool isExplain = true; - makeUpdateRequest(opCtx, args, isExplain, &updateLifecycle, &request); + makeUpdateRequest(opCtx, args, isExplain, &request); ParsedUpdate parsedUpdate(opCtx, &request); uassertStatusOK(parsedUpdate.parseRequest()); @@ -422,7 +393,7 @@ public: opDebug->setPlanSummaryMetrics(summaryStats); // Fill out OpDebug with the number of deleted docs. - opDebug->additiveMetrics.ndeleted = getDeleteStats(exec.get())->docsDeleted; + opDebug->additiveMetrics.ndeleted = DeleteStage::getNumDeleted(*exec); if (curOp->shouldDBProfile()) { BSONObjBuilder execStatsBob; @@ -434,9 +405,8 @@ public: appendCommandResponse(exec.get(), args.isRemove(), docFound, &result); } else { UpdateRequest request(nsString); - UpdateLifecycleImpl updateLifecycle(nsString); const bool isExplain = false; - makeUpdateRequest(opCtx, args, isExplain, &updateLifecycle, &request); + makeUpdateRequest(opCtx, args, isExplain, &request); if (opCtx->getTxnNumber()) { request.setStmtId(stmtId); @@ -517,7 +487,8 @@ public: if (collection) { collection->infoCache()->notifyOfQuery(opCtx, summaryStats.indexesUsed); } - UpdateStage::recordUpdateStatsInOpDebug(getUpdateStats(exec.get()), opDebug); + UpdateStage::recordUpdateStatsInOpDebug(UpdateStage::getUpdateStats(exec.get()), + opDebug); opDebug->setPlanSummaryMetrics(summaryStats); if (curOp->shouldDBProfile()) { diff --git a/src/mongo/db/commands/write_commands/write_commands.cpp b/src/mongo/db/commands/write_commands/write_commands.cpp index 80e51c8101b..5d980e5814c 100644 --- a/src/mongo/db/commands/write_commands/write_commands.cpp +++ b/src/mongo/db/commands/write_commands/write_commands.cpp @@ -43,7 +43,6 @@ #include "mongo/db/ops/delete_request.h" #include "mongo/db/ops/parsed_delete.h" #include "mongo/db/ops/parsed_update.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/ops/write_ops.h" #include "mongo/db/ops/write_ops_exec.h" #include "mongo/db/query/explain.h" @@ -357,9 +356,7 @@ private: "explained write batches must be of size 1", _batch.getUpdates().size() == 1); - UpdateLifecycleImpl updateLifecycle(_batch.getNamespace()); UpdateRequest updateRequest(_batch.getNamespace()); - updateRequest.setLifecycle(&updateLifecycle); updateRequest.setQuery(_batch.getUpdates()[0].getQ()); updateRequest.setUpdates(_batch.getUpdates()[0].getU()); updateRequest.setCollation(write_ops::collationOf(_batch.getUpdates()[0])); diff --git a/src/mongo/db/dbhelpers.cpp b/src/mongo/db/dbhelpers.cpp index 58d26fca659..4c547d68a7e 100644 --- a/src/mongo/db/dbhelpers.cpp +++ b/src/mongo/db/dbhelpers.cpp @@ -47,7 +47,6 @@ #include "mongo/db/op_observer.h" #include "mongo/db/ops/delete.h" #include "mongo/db/ops/update.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/ops/update_request.h" #include "mongo/db/ops/update_result.h" #include "mongo/db/query/get_executor.h" @@ -236,8 +235,6 @@ void Helpers::upsert(OperationContext* opCtx, request.setUpdates(o); request.setUpsert(); request.setFromMigration(fromMigrate); - UpdateLifecycleImpl updateLifecycle(requestNs); - request.setLifecycle(&updateLifecycle); update(opCtx, context.db(), request); } @@ -250,8 +247,6 @@ void Helpers::putSingleton(OperationContext* opCtx, const char* ns, BSONObj obj) request.setUpdates(obj); request.setUpsert(); - UpdateLifecycleImpl updateLifecycle(requestNs); - request.setLifecycle(&updateLifecycle); update(opCtx, context.db(), request); diff --git a/src/mongo/db/exec/delete.cpp b/src/mongo/db/exec/delete.cpp index 3b8a6f803c6..a279cdd27f7 100644 --- a/src/mongo/db/exec/delete.cpp +++ b/src/mongo/db/exec/delete.cpp @@ -78,19 +78,15 @@ DeleteStage::DeleteStage(OperationContext* opCtx, WorkingSet* ws, Collection* collection, PlanStage* child) - : PlanStage(kStageType, opCtx), + : RequiresMutableCollectionStage(kStageType, opCtx, collection), _params(params), _ws(ws), - _collection(collection), _idRetrying(WorkingSet::INVALID_ID), _idReturning(WorkingSet::INVALID_ID) { _children.emplace_back(child); } bool DeleteStage::isEOF() { - if (!_collection) { - return true; - } if (!_params.isMulti && _specificStats.docsDeleted > 0) { return true; } @@ -102,7 +98,6 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { if (isEOF()) { return PlanStage::IS_EOF; } - invariant(_collection); // If isEOF() returns false, we must have a collection. // It is possible that after a delete was executed, a WriteConflictException occurred // and prevented us from returning ADVANCED with the old version of the document. @@ -169,7 +164,7 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { bool docStillMatches; try { docStillMatches = write_stage_common::ensureStillMatches( - _collection, getOpCtx(), _ws, id, _params.canonicalQuery); + collection(), getOpCtx(), _ws, id, _params.canonicalQuery); } catch (const WriteConflictException&) { // There was a problem trying to detect if the document still exists, so retry. memberFreer.Dismiss(); @@ -208,14 +203,14 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { if (!_params.isExplain) { try { WriteUnitOfWork wunit(getOpCtx()); - _collection->deleteDocument(getOpCtx(), - _params.stmtId, - recordId, - _params.opDebug, - _params.fromMigrate, - false, - _params.returnDeleted ? Collection::StoreDeletedDoc::On - : Collection::StoreDeletedDoc::Off); + collection()->deleteDocument(getOpCtx(), + _params.stmtId, + recordId, + _params.opDebug, + _params.fromMigrate, + false, + _params.returnDeleted ? Collection::StoreDeletedDoc::On + : Collection::StoreDeletedDoc::Off); wunit.commit(); } catch (const WriteConflictException&) { memberFreer.Dismiss(); // Keep this member around so we can retry deleting it. @@ -263,9 +258,8 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { return PlanStage::NEED_TIME; } -void DeleteStage::doRestoreState() { - invariant(_collection); - const NamespaceString& ns(_collection->ns()); +void DeleteStage::restoreState(RequiresCollTag) { + const NamespaceString& ns = collection()->ns(); uassert(ErrorCodes::PrimarySteppedDown, str::stream() << "Demoted from primary while removing from " << ns.ns(), !getOpCtx()->writesAreReplicated() || @@ -287,11 +281,26 @@ const SpecificStats* DeleteStage::getSpecificStats() const { // static long long DeleteStage::getNumDeleted(const PlanExecutor& exec) { invariant(exec.getRootStage()->isEOF()); - invariant(exec.getRootStage()->stageType() == STAGE_DELETE); - DeleteStage* deleteStage = static_cast<DeleteStage*>(exec.getRootStage()); - const DeleteStats* deleteStats = - static_cast<const DeleteStats*>(deleteStage->getSpecificStats()); - return deleteStats->docsDeleted; + + // If we're deleting from a non-existent collection, then the delete plan may have an EOF as the + // root stage. + if (exec.getRootStage()->stageType() == STAGE_EOF) { + return 0LL; + } + + // If the collection exists, the delete plan may either have a delete stage at the root, or (for + // findAndModify) a projection stage wrapping a delete stage. + if (StageType::STAGE_PROJECTION == exec.getRootStage()->stageType()) { + invariant(exec.getRootStage()->getChildren().size() == 1U); + invariant(StageType::STAGE_DELETE == exec.getRootStage()->child()->stageType()); + const SpecificStats* stats = exec.getRootStage()->child()->getSpecificStats(); + return static_cast<const DeleteStats*>(stats)->docsDeleted; + } else { + invariant(StageType::STAGE_DELETE == exec.getRootStage()->stageType()); + const auto* deleteStats = + static_cast<const DeleteStats*>(exec.getRootStage()->getSpecificStats()); + return deleteStats->docsDeleted; + } } PlanStage::StageState DeleteStage::prepareToRetryWSM(WorkingSetID idToRetry, WorkingSetID* out) { diff --git a/src/mongo/db/exec/delete.h b/src/mongo/db/exec/delete.h index cfd65c8acec..c3fd5ee5cba 100644 --- a/src/mongo/db/exec/delete.h +++ b/src/mongo/db/exec/delete.h @@ -30,7 +30,7 @@ #pragma once -#include "mongo/db/exec/plan_stage.h" +#include "mongo/db/exec/requires_collection_stage.h" #include "mongo/db/jsobj.h" #include "mongo/db/logical_session_id.h" @@ -85,7 +85,7 @@ struct DeleteStageParams { * Callers of work() must be holding a write lock (and, for replicated deletes, callers must have * had the replication coordinator approve the write). */ -class DeleteStage final : public PlanStage { +class DeleteStage final : public RequiresMutableCollectionStage { MONGO_DISALLOW_COPYING(DeleteStage); public: @@ -98,8 +98,6 @@ public: bool isEOF() final; StageState doWork(WorkingSetID* out) final; - void doRestoreState() final; - StageType stageType() const final { return STAGE_DELETE; } @@ -117,6 +115,11 @@ public: */ static long long getNumDeleted(const PlanExecutor& exec); +protected: + void saveState(RequiresCollTag) final {} + + void restoreState(RequiresCollTag) final; + private: /** * Stores 'idToRetry' in '_idRetrying' so the delete can be retried during the next call to @@ -129,11 +132,6 @@ private: // Not owned by us. WorkingSet* _ws; - // Collection to operate on. Not owned by us. Can be NULL (if NULL, isEOF() will always - // return true). If non-NULL, the lifetime of the collection must supersede that of the - // stage. - Collection* _collection; - // If not WorkingSet::INVALID_ID, we use this rather than asking our child what to do next. WorkingSetID _idRetrying; diff --git a/src/mongo/db/exec/requires_collection_stage.cpp b/src/mongo/db/exec/requires_collection_stage.cpp index 106767eed73..9424f542839 100644 --- a/src/mongo/db/exec/requires_collection_stage.cpp +++ b/src/mongo/db/exec/requires_collection_stage.cpp @@ -35,14 +35,16 @@ namespace mongo { -void RequiresCollectionStage::doSaveState() { +template <typename CollectionT> +void RequiresCollectionStageBase<CollectionT>::doSaveState() { // A stage may not access storage while in a saved state. _collection = nullptr; saveState(RequiresCollTag{}); } -void RequiresCollectionStage::doRestoreState() { +template <typename CollectionT> +void RequiresCollectionStageBase<CollectionT>::doRestoreState() { invariant(!_collection); const UUIDCatalog& catalog = UUIDCatalog::get(getOpCtx()); @@ -54,4 +56,7 @@ void RequiresCollectionStage::doRestoreState() { restoreState(RequiresCollTag{}); } +template class RequiresCollectionStageBase<const Collection*>; +template class RequiresCollectionStageBase<Collection*>; + } // namespace mongo diff --git a/src/mongo/db/exec/requires_collection_stage.h b/src/mongo/db/exec/requires_collection_stage.h index f6192331374..273a7185ae4 100644 --- a/src/mongo/db/exec/requires_collection_stage.h +++ b/src/mongo/db/exec/requires_collection_stage.h @@ -43,17 +43,24 @@ namespace mongo { * * Subclasses must implement the saveStage() and restoreState() variants tagged with RequiresCollTag * in order to supply custom yield preparation or yield recovery logic. + * + * Templated on 'CollectionT', which may be instantiated using either Collection* or const + * Collection*. This abstracts the implementation of this base class for use by derived classes + * which read (e.g. COLLSCAN and MULTI_ITERATOR) and derived classes that write (e.g. UPDATE and + * DELETE). Derived classes should use the 'RequiresCollectionStage' or + * 'RequiresMutableCollectionStage' aliases provided below. */ -class RequiresCollectionStage : public PlanStage { +template <typename CollectionT> +class RequiresCollectionStageBase : public PlanStage { public: - RequiresCollectionStage(const char* stageType, OperationContext* opCtx, const Collection* coll) + RequiresCollectionStageBase(const char* stageType, OperationContext* opCtx, CollectionT coll) : PlanStage(stageType, opCtx), _collection(coll), _collectionUUID(_collection->uuid().get()) { invariant(_collection); } - virtual ~RequiresCollectionStage() = default; + virtual ~RequiresCollectionStageBase() = default; protected: struct RequiresCollTag {}; @@ -72,17 +79,23 @@ protected: */ virtual void restoreState(RequiresCollTag) = 0; - const Collection* collection() { + CollectionT collection() const { return _collection; } - UUID uuid() { + UUID uuid() const { return _collectionUUID; } private: - const Collection* _collection; + CollectionT _collection; const UUID _collectionUUID; }; +// Type alias for use by PlanStages that read a Collection. +using RequiresCollectionStage = RequiresCollectionStageBase<const Collection*>; + +// Type alias for use by PlanStages that write to a Collection. +using RequiresMutableCollectionStage = RequiresCollectionStageBase<Collection*>; + } // namespace mongo diff --git a/src/mongo/db/exec/update.cpp b/src/mongo/db/exec/update.cpp index ab14a670f1f..d1f0a3aab77 100644 --- a/src/mongo/db/exec/update.cpp +++ b/src/mongo/db/exec/update.cpp @@ -43,7 +43,6 @@ #include "mongo/db/exec/working_set_common.h" #include "mongo/db/exec/write_stage_common.h" #include "mongo/db/op_observer.h" -#include "mongo/db/ops/update_lifecycle.h" #include "mongo/db/query/explain.h" #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/s/collection_sharding_state.h" @@ -161,15 +160,16 @@ CollectionUpdateArgs::StoreDocOption getStoreDocMode(const UpdateRequest& update const char* UpdateStage::kStageType = "UPDATE"; +const UpdateStats UpdateStage::kEmptyUpdateStats; + UpdateStage::UpdateStage(OperationContext* opCtx, const UpdateStageParams& params, WorkingSet* ws, Collection* collection, PlanStage* child) - : PlanStage(kStageType, opCtx), + : RequiresMutableCollectionStage(kStageType, opCtx, collection), _params(params), _ws(ws), - _collection(collection), _idRetrying(WorkingSet::INVALID_ID), _idReturning(WorkingSet::INVALID_ID), _updatedRecordIds(params.request->isMulti() ? new RecordIdSet() : NULL), @@ -193,7 +193,6 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco const UpdateRequest* request = _params.request; UpdateDriver* driver = _params.driver; CanonicalQuery* cq = _params.canonicalQuery; - UpdateLifecycle* lifecycle = request->getLifecycle(); // If asked to return new doc, default to the oldObj, in case nothing changes. BSONObj newObj = oldObj.value(); @@ -205,7 +204,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco // only enable in-place mutations if the underlying storage engine offers support for // writing damage events. _doc.reset(oldObj.value(), - (_collection->updateWithDamagesSupported() + (collection()->updateWithDamagesSupported() ? mutablebson::Document::kInPlaceEnabled : mutablebson::Document::kInPlaceDisabled)); @@ -217,13 +216,10 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco const bool validateForStorage = getOpCtx()->writesAreReplicated() && _enforceOkForStorage; FieldRefSet immutablePaths; if (getOpCtx()->writesAreReplicated() && !request->isFromMigration()) { - if (lifecycle) { - auto immutablePathsVector = - getImmutableFields(getOpCtx(), request->getNamespaceString()); - if (immutablePathsVector) { - immutablePaths.fillFrom( - transitional_tools_do_not_use::unspool_vector(*immutablePathsVector)); - } + auto immutablePathsVector = getImmutableFields(getOpCtx(), request->getNamespaceString()); + if (immutablePathsVector) { + immutablePaths.fillFrom( + transitional_tools_do_not_use::unspool_vector(*immutablePathsVector)); } immutablePaths.keepShortest(&idFieldRef); } @@ -253,7 +249,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco // Skip adding _id field if the collection is capped (since capped collection documents can // neither grow nor shrink). - const auto createIdField = !_collection->isCapped(); + const auto createIdField = !collection()->isCapped(); // Ensure if _id exists it is first status = ensureIdFieldIsFirst(&_doc); @@ -288,8 +284,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco RecordId newRecordId; CollectionUpdateArgs args; if (!request->isExplain()) { - invariant(_collection); - auto* css = CollectionShardingState::get(getOpCtx(), _collection->ns()); + auto* css = CollectionShardingState::get(getOpCtx(), collection()->ns()); args.stmtId = request->getStmtId(); args.update = logObj; auto metadata = css->getMetadata(getOpCtx()); @@ -311,7 +306,7 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco Snapshotted<RecordData> snap(oldObj.snapshotId(), oldRec); - StatusWith<RecordData> newRecStatus = _collection->updateDocumentWithDamages( + StatusWith<RecordData> newRecStatus = collection()->updateDocumentWithDamages( getOpCtx(), recordId, std::move(snap), source, _damages, &args); newObj = uassertStatusOK(std::move(newRecStatus)).releaseToBson(); @@ -328,13 +323,13 @@ BSONObj UpdateStage::transformAndUpdate(const Snapshotted<BSONObj>& oldObj, Reco newObj.objsize() <= BSONObjMaxUserSize); if (!request->isExplain()) { - newRecordId = _collection->updateDocument(getOpCtx(), - recordId, - oldObj, - newObj, - driver->modsAffectIndices(), - _params.opDebug, - &args); + newRecordId = collection()->updateDocument(getOpCtx(), + recordId, + oldObj, + newObj, + driver->modsAffectIndices(), + _params.opDebug, + &args); } } @@ -463,13 +458,12 @@ void UpdateStage::doInsert() { return; } - writeConflictRetry(getOpCtx(), "upsert", _collection->ns().ns(), [&] { + writeConflictRetry(getOpCtx(), "upsert", collection()->ns().ns(), [&] { WriteUnitOfWork wunit(getOpCtx()); - invariant(_collection); - uassertStatusOK(_collection->insertDocument(getOpCtx(), - InsertStatement(request->getStmtId(), newObj), - _params.opDebug, - request->isFromMigration())); + uassertStatusOK(collection()->insertDocument(getOpCtx(), + InsertStatement(request->getStmtId(), newObj), + _params.opDebug, + request->isFromMigration())); // Technically, we should save/restore state here, but since we are going to return // immediately after, it would just be wasted work. @@ -531,10 +525,6 @@ PlanStage::StageState UpdateStage::doWork(WorkingSetID* out) { return PlanStage::IS_EOF; } - // If we're here, then we still have to ask for results from the child and apply - // updates to them. We should only get here if the collection exists. - invariant(_collection); - // It is possible that after an update was applied, a WriteConflictException // occurred and prevented us from returning ADVANCED with the requested version // of the document. @@ -589,7 +579,7 @@ PlanStage::StageState UpdateStage::doWork(WorkingSetID* out) { bool docStillMatches; try { docStillMatches = write_stage_common::ensureStillMatches( - _collection, getOpCtx(), _ws, id, _params.canonicalQuery); + collection(), getOpCtx(), _ws, id, _params.canonicalQuery); } catch (const WriteConflictException&) { // There was a problem trying to detect if the document still exists, so retry. memberFreer.Dismiss(); @@ -702,7 +692,7 @@ PlanStage::StageState UpdateStage::doWork(WorkingSetID* out) { return status; } -void UpdateStage::doRestoreState() { +void UpdateStage::restoreState(RequiresCollTag) { const UpdateRequest& request = *_params.request; const NamespaceString& nsString(request.getNamespaceString()); @@ -716,16 +706,10 @@ void UpdateStage::doRestoreState() { << nsString.ns()); } - if (request.getLifecycle()) { - UpdateLifecycle* lifecycle = request.getLifecycle(); - lifecycle->setCollection(_collection); - - if (!lifecycle->canContinue()) { - uasserted(17270, "Update aborted due to invalid state transitions after yield."); - } - - _params.driver->refreshIndexKeys(lifecycle->getIndexKeys(getOpCtx())); - } + // The set of indices may have changed during yield. Make sure that the update driver has up to + // date index information. + const auto& updateIndexData = collection()->infoCache()->getIndexKeys(getOpCtx()); + _params.driver->refreshIndexKeys(&updateIndexData); } unique_ptr<PlanStageStats> UpdateStage::getStats() { @@ -742,9 +726,24 @@ const SpecificStats* UpdateStage::getSpecificStats() const { const UpdateStats* UpdateStage::getUpdateStats(const PlanExecutor* exec) { invariant(exec->getRootStage()->isEOF()); - invariant(exec->getRootStage()->stageType() == STAGE_UPDATE); - UpdateStage* updateStage = static_cast<UpdateStage*>(exec->getRootStage()); - return static_cast<const UpdateStats*>(updateStage->getSpecificStats()); + + // If we're updating a non-existent collection, then the delete plan may have an EOF as the root + // stage. + if (exec->getRootStage()->stageType() == STAGE_EOF) { + return &kEmptyUpdateStats; + } + + // If the collection exists, then we expect the root of the plan tree to either be an update + // stage, or (for findAndModify) a projection stage wrapping an update stage. + if (StageType::STAGE_PROJECTION == exec->getRootStage()->stageType()) { + invariant(exec->getRootStage()->getChildren().size() == 1U); + invariant(StageType::STAGE_UPDATE == exec->getRootStage()->child()->stageType()); + const SpecificStats* stats = exec->getRootStage()->child()->getSpecificStats(); + return static_cast<const UpdateStats*>(stats); + } else { + invariant(StageType::STAGE_UPDATE == exec->getRootStage()->stageType()); + return static_cast<const UpdateStats*>(exec->getRootStage()->getSpecificStats()); + } } void UpdateStage::recordUpdateStatsInOpDebug(const UpdateStats* updateStats, OpDebug* opDebug) { diff --git a/src/mongo/db/exec/update.h b/src/mongo/db/exec/update.h index 6a988609162..21ee75e2612 100644 --- a/src/mongo/db/exec/update.h +++ b/src/mongo/db/exec/update.h @@ -32,7 +32,7 @@ #include "mongo/db/catalog/collection.h" -#include "mongo/db/exec/plan_stage.h" +#include "mongo/db/exec/requires_collection_stage.h" #include "mongo/db/jsobj.h" #include "mongo/db/ops/update_request.h" #include "mongo/db/ops/update_result.h" @@ -75,7 +75,7 @@ private: * * Callers of work() must be holding a write lock. */ -class UpdateStage final : public PlanStage { +class UpdateStage final : public RequiresMutableCollectionStage { MONGO_DISALLOW_COPYING(UpdateStage); public: @@ -88,8 +88,6 @@ public: bool isEOF() final; StageState doWork(WorkingSetID* out) final; - void doRestoreState() final; - StageType stageType() const final { return STAGE_UPDATE; } @@ -146,7 +144,14 @@ public: bool enforceOkForStorage, UpdateStats* stats); +protected: + void saveState(RequiresCollTag) final {} + + void restoreState(RequiresCollTag) final; + private: + static const UpdateStats kEmptyUpdateStats; + /** * Computes the result of applying mods to the document 'oldObj' at RecordId 'recordId' in * memory, then commits these changes to the database. Returns a possibly unowned copy @@ -183,9 +188,6 @@ private: // Not owned by us. WorkingSet* _ws; - // Not owned by us. May be NULL. - Collection* _collection; - // If not WorkingSet::INVALID_ID, we use this rather than asking our child what to do next. WorkingSetID _idRetrying; diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp index 2774cb24cd0..ff5ad3b307d 100644 --- a/src/mongo/db/ops/update.cpp +++ b/src/mongo/db/ops/update.cpp @@ -45,7 +45,6 @@ #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/exec/update.h" #include "mongo/db/op_observer.h" -#include "mongo/db/ops/update_lifecycle.h" #include "mongo/db/query/explain.h" #include "mongo/db/query/get_executor.h" #include "mongo/db/query/plan_summary_stats.h" diff --git a/src/mongo/db/ops/update_lifecycle.h b/src/mongo/db/ops/update_lifecycle.h deleted file mode 100644 index a2da7e4d805..00000000000 --- a/src/mongo/db/ops/update_lifecycle.h +++ /dev/null @@ -1,64 +0,0 @@ - -/** - * Copyright (C) 2018-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * As a special exception, the copyright holders give permission to link the - * code of portions of this program with the OpenSSL library under certain - * conditions as described in each individual source file and distribute - * linked combinations including the program with the OpenSSL library. You - * must comply with the Server Side Public License in all respects for - * all of the code used other than as permitted herein. If you modify file(s) - * with this exception, you may extend this exception to your version of the - * file(s), but you are not obligated to do so. If you do not wish to do so, - * delete this exception statement from your version. If you delete this - * exception statement from all source files in the program, then also delete - * it in the license file. - */ - -#pragma once - -namespace mongo { - -class Collection; -class OperationContext; -class UpdateIndexData; - - -class UpdateLifecycle { -public: - virtual ~UpdateLifecycle() {} - - /** - * Update the cached collection pointer that this lifecycle object uses. - */ - virtual void setCollection(Collection* collection) = 0; - - /** - * Can the update continue? - * - * The (only) implementation will check the following: - * 1.) Collection still exists - * 2.) Shard version has not changed (indicating that the query/update is not valid - */ - virtual bool canContinue() const = 0; - - /** - * Return a pointer to any indexes if there is a collection. - */ - virtual const UpdateIndexData* getIndexKeys(OperationContext* opCtx) const = 0; -}; - -} // namespace mongo diff --git a/src/mongo/db/ops/update_lifecycle_impl.cpp b/src/mongo/db/ops/update_lifecycle_impl.cpp deleted file mode 100644 index 5921a27673b..00000000000 --- a/src/mongo/db/ops/update_lifecycle_impl.cpp +++ /dev/null @@ -1,57 +0,0 @@ - -/** - * Copyright (C) 2018-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * As a special exception, the copyright holders give permission to link the - * code of portions of this program with the OpenSSL library under certain - * conditions as described in each individual source file and distribute - * linked combinations including the program with the OpenSSL library. You - * must comply with the Server Side Public License in all respects for - * all of the code used other than as permitted herein. If you modify file(s) - * with this exception, you may extend this exception to your version of the - * file(s), but you are not obligated to do so. If you do not wish to do so, - * delete this exception statement from your version. If you delete this - * exception statement from all source files in the program, then also delete - * it in the license file. - */ - -#include "mongo/platform/basic.h" - -#include "mongo/db/ops/update_lifecycle_impl.h" - -#include "mongo/db/catalog/collection.h" -#include "mongo/db/field_ref.h" - -namespace mongo { - -UpdateLifecycleImpl::UpdateLifecycleImpl(const NamespaceString& nsStr) : _nsString(nsStr) {} - -void UpdateLifecycleImpl::setCollection(Collection* collection) { - _collection = collection; -} - -bool UpdateLifecycleImpl::canContinue() const { - // Collection needs to exist to continue - return _collection; -} - -const UpdateIndexData* UpdateLifecycleImpl::getIndexKeys(OperationContext* opCtx) const { - if (_collection) - return &_collection->infoCache()->getIndexKeys(opCtx); - return NULL; -} - -} // namespace mongo diff --git a/src/mongo/db/ops/update_lifecycle_impl.h b/src/mongo/db/ops/update_lifecycle_impl.h deleted file mode 100644 index f5608a70fbb..00000000000 --- a/src/mongo/db/ops/update_lifecycle_impl.h +++ /dev/null @@ -1,66 +0,0 @@ - -/** - * Copyright (C) 2018-present MongoDB, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the Server Side Public License, version 1, - * as published by MongoDB, Inc. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * Server Side Public License for more details. - * - * You should have received a copy of the Server Side Public License - * along with this program. If not, see - * <http://www.mongodb.com/licensing/server-side-public-license>. - * - * As a special exception, the copyright holders give permission to link the - * code of portions of this program with the OpenSSL library under certain - * conditions as described in each individual source file and distribute - * linked combinations including the program with the OpenSSL library. You - * must comply with the Server Side Public License in all respects for - * all of the code used other than as permitted herein. If you modify file(s) - * with this exception, you may extend this exception to your version of the - * file(s), but you are not obligated to do so. If you do not wish to do so, - * delete this exception statement from your version. If you delete this - * exception statement from all source files in the program, then also delete - * it in the license file. - */ - -#pragma once - -#include "mongo/base/disallow_copying.h" -#include "mongo/db/namespace_string.h" -#include "mongo/db/ops/update_lifecycle.h" -#include "mongo/s/chunk_version.h" - -namespace mongo { - -class Collection; - -class UpdateLifecycleImpl : public UpdateLifecycle { - MONGO_DISALLOW_COPYING(UpdateLifecycleImpl); - -public: - /** - * ignoreVersion is for shard version checking and - * means that version checks will not be done - * - * nsString represents the namespace for the - */ - UpdateLifecycleImpl(const NamespaceString& nsString); - - virtual void setCollection(Collection* collection); - - virtual bool canContinue() const; - - virtual const UpdateIndexData* getIndexKeys(OperationContext* opCtx) const; - -private: - const NamespaceString& _nsString; - - Collection* _collection; -}; - -} /* namespace mongo */ diff --git a/src/mongo/db/ops/update_request.h b/src/mongo/db/ops/update_request.h index 3fcfcf11cd3..af6d2aa7a01 100644 --- a/src/mongo/db/ops/update_request.h +++ b/src/mongo/db/ops/update_request.h @@ -42,7 +42,6 @@ namespace mongo { namespace str = mongoutils::str; class FieldRef; -class UpdateLifecycle; class UpdateRequest { public: @@ -65,7 +64,6 @@ public: _multi(false), _fromMigration(false), _fromOplogApplication(false), - _lifecycle(NULL), _isExplain(false), _returnDocs(ReturnDocOption::RETURN_NONE), _yieldPolicy(PlanExecutor::NO_YIELD) {} @@ -165,14 +163,6 @@ public: return _fromOplogApplication; } - inline void setLifecycle(UpdateLifecycle* value) { - _lifecycle = value; - } - - inline UpdateLifecycle* getLifecycle() const { - return _lifecycle; - } - inline void setExplain(bool value = true) { _isExplain = value; } @@ -284,9 +274,6 @@ private: // True if this update was triggered by the application of an oplog entry. bool _fromOplogApplication; - // The lifecycle data, and events used during the update request. - UpdateLifecycle* _lifecycle; - // Whether or not we are requesting an explained update. Explained updates are read-only. bool _isExplain; diff --git a/src/mongo/db/ops/write_ops_exec.cpp b/src/mongo/db/ops/write_ops_exec.cpp index 834a2034033..b205ebbb26b 100644 --- a/src/mongo/db/ops/write_ops_exec.cpp +++ b/src/mongo/db/ops/write_ops_exec.cpp @@ -54,7 +54,6 @@ #include "mongo/db/ops/insert.h" #include "mongo/db/ops/parsed_delete.h" #include "mongo/db/ops/parsed_update.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/ops/update_request.h" #include "mongo/db/ops/write_ops_exec.h" #include "mongo/db/ops/write_ops_gen.h" @@ -575,9 +574,7 @@ static SingleWriteResult performSingleUpdateOp(OperationContext* opCtx, curOp.ensureStarted(); } - UpdateLifecycleImpl updateLifecycle(ns); UpdateRequest request(ns); - request.setLifecycle(&updateLifecycle); request.setQuery(op.getQ()); request.setUpdates(op.getU()); request.setCollation(write_ops::collationOf(op)); diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 4e8f8889c04..14c2c137c88 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -59,7 +59,6 @@ #include "mongo/db/index_names.h" #include "mongo/db/matcher/extensions_callback_noop.h" #include "mongo/db/matcher/extensions_callback_real.h" -#include "mongo/db/ops/update_lifecycle.h" #include "mongo/db/query/canonical_query.h" #include "mongo/db/query/canonical_query_encoder.h" #include "mongo/db/query/collation/collator_factory_interface.h" @@ -821,22 +820,20 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDelete( unique_ptr<WorkingSet> ws = make_unique<WorkingSet>(); const PlanExecutor::YieldPolicy policy = parsedDelete->yieldPolicy(); + if (!collection) { + // Treat collections that do not exist as empty collections. Return a PlanExecutor which + // contains an EOF stage. + LOG(2) << "Collection " << nss.ns() << " does not exist." + << " Using EOF stage: " << redact(request->getQuery()); + return PlanExecutor::make( + opCtx, std::move(ws), std::make_unique<EOFStage>(opCtx), nss, policy); + } + if (!parsedDelete->hasParsedQuery()) { - // This is the idhack fast-path for getting a PlanExecutor without doing the work - // to create a CanonicalQuery. + // This is the idhack fast-path for getting a PlanExecutor without doing the work to create + // a CanonicalQuery. const BSONObj& unparsedQuery = request->getQuery(); - if (!collection) { - // Treat collections that do not exist as empty collections. Note that the explain - // reporting machinery always assumes that the root stage for a delete operation is - // a DeleteStage, so in this case we put a DeleteStage on top of an EOFStage. - LOG(2) << "Collection " << nss.ns() << " does not exist." - << " Using EOF stage: " << redact(unparsedQuery); - auto deleteStage = make_unique<DeleteStage>( - opCtx, deleteStageParams, ws.get(), nullptr, new EOFStage(opCtx)); - return PlanExecutor::make(opCtx, std::move(ws), std::move(deleteStage), nss, policy); - } - const IndexDescriptor* descriptor = collection->getIndexCatalog()->findIdIndex(opCtx); // Construct delete request collator. @@ -922,7 +919,6 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorUpdate( UpdateDriver* driver = parsedUpdate->getDriver(); const NamespaceString& nss = request->getNamespaceString(); - UpdateLifecycle* lifecycle = request->getLifecycle(); if (nss.isSystem() && opCtx->lockState()->shouldConflictWithSecondaryBatchApplication()) { uassert(10156, @@ -959,32 +955,32 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorUpdate( str::stream() << "Not primary while performing update on " << nss.ns()); } - if (lifecycle) { - lifecycle->setCollection(collection); - driver->refreshIndexKeys(lifecycle->getIndexKeys(opCtx)); - } - const PlanExecutor::YieldPolicy policy = parsedUpdate->yieldPolicy(); unique_ptr<WorkingSet> ws = make_unique<WorkingSet>(); UpdateStageParams updateStageParams(request, driver, opDebug); + // If the collection doesn't exist, then return a PlanExecutor for a no-op EOF plan. We have + // should have already enforced upstream that in this case either the upsert flag is false, or + // we are an explain. If the collection doesn't exist, we're not an explain, and the upsert flag + // is true, we expect the caller to have created the collection already. + if (!collection) { + LOG(2) << "Collection " << nss.ns() << " does not exist." + << " Using EOF stage: " << redact(request->getQuery()); + return PlanExecutor::make( + opCtx, std::move(ws), std::make_unique<EOFStage>(opCtx), nss, policy); + } + + // Pass index information to the update driver, so that it can determine for us whether the + // update affects indices. + const auto& updateIndexData = collection->infoCache()->getIndexKeys(opCtx); + driver->refreshIndexKeys(&updateIndexData); + if (!parsedUpdate->hasParsedQuery()) { // This is the idhack fast-path for getting a PlanExecutor without doing the work // to create a CanonicalQuery. const BSONObj& unparsedQuery = request->getQuery(); - if (!collection) { - // Treat collections that do not exist as empty collections. Note that the explain - // reporting machinery always assumes that the root stage for an update operation is - // an UpdateStage, so in this case we put an UpdateStage on top of an EOFStage. - LOG(2) << "Collection " << nss.ns() << " does not exist." - << " Using EOF stage: " << redact(unparsedQuery); - auto updateStage = make_unique<UpdateStage>( - opCtx, updateStageParams, ws.get(), collection, new EOFStage(opCtx)); - return PlanExecutor::make(opCtx, std::move(ws), std::move(updateStage), nss, policy); - } - const IndexDescriptor* descriptor = collection->getIndexCatalog()->findIdIndex(opCtx); const bool hasCollectionDefaultCollation = CollatorInterface::collatorsMatch( diff --git a/src/mongo/db/query/internal_plans.cpp b/src/mongo/db/query/internal_plans.cpp index 638dcf68662..dff69fc1298 100644 --- a/src/mongo/db/query/internal_plans.cpp +++ b/src/mongo/db/query/internal_plans.cpp @@ -82,6 +82,7 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> InternalPlanner::deleteWith PlanExecutor::YieldPolicy yieldPolicy, Direction direction, const RecordId& startLoc) { + invariant(collection); auto ws = stdx::make_unique<WorkingSet>(); auto root = _collectionScan(opCtx, ws.get(), collection, direction, startLoc); @@ -133,6 +134,7 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> InternalPlanner::deleteWith BoundInclusion boundInclusion, PlanExecutor::YieldPolicy yieldPolicy, Direction direction) { + invariant(collection); auto ws = stdx::make_unique<WorkingSet>(); std::unique_ptr<PlanStage> root = _indexScan(opCtx, @@ -160,6 +162,7 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> InternalPlanner::updateWith const IndexDescriptor* descriptor, const BSONObj& key, PlanExecutor::YieldPolicy yieldPolicy) { + invariant(collection); auto ws = stdx::make_unique<WorkingSet>(); auto idHackStage = stdx::make_unique<IDHackStage>(opCtx, collection, key, ws.get(), descriptor); diff --git a/src/mongo/db/repl/oplog.cpp b/src/mongo/db/repl/oplog.cpp index 9cb6367b75c..8b0f9d7181e 100644 --- a/src/mongo/db/repl/oplog.cpp +++ b/src/mongo/db/repl/oplog.cpp @@ -72,7 +72,6 @@ #include "mongo/db/op_observer.h" #include "mongo/db/ops/delete.h" #include "mongo/db/ops/update.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/repl/apply_ops.h" #include "mongo/db/repl/bgsync.h" #include "mongo/db/repl/dbcheck.h" @@ -1370,9 +1369,6 @@ Status applyOperation_inlock(OperationContext* opCtx, request.setUpsert(); request.setFromOplogApplication(true); - UpdateLifecycleImpl updateLifecycle(requestNss); - request.setLifecycle(&updateLifecycle); - const StringData ns = fieldNs.valuestrsafe(); writeConflictRetry(opCtx, "applyOps_upsert", ns, [&] { WriteUnitOfWork wuow(opCtx); @@ -1415,9 +1411,6 @@ Status applyOperation_inlock(OperationContext* opCtx, request.setUpsert(upsert); request.setFromOplogApplication(true); - UpdateLifecycleImpl updateLifecycle(requestNss); - request.setLifecycle(&updateLifecycle); - Timestamp timestamp; if (assignOperationTimestamp) { timestamp = fieldTs.timestamp(); diff --git a/src/mongo/db/repl/rs_rollback.cpp b/src/mongo/db/repl/rs_rollback.cpp index 1927ffd20ea..86dd5ee7199 100644 --- a/src/mongo/db/repl/rs_rollback.cpp +++ b/src/mongo/db/repl/rs_rollback.cpp @@ -57,7 +57,6 @@ #include "mongo/db/logical_time_validator.h" #include "mongo/db/ops/delete.h" #include "mongo/db/ops/update.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/ops/update_request.h" #include "mongo/db/query/internal_plans.h" #include "mongo/db/repl/bgsync.h" @@ -1392,8 +1391,6 @@ void rollback_internal::syncFixUp(OperationContext* opCtx, request.setUpdates(idAndDoc.second); request.setGod(); request.setUpsert(); - UpdateLifecycleImpl updateLifecycle(nss); - request.setLifecycle(&updateLifecycle); update(opCtx, ctx.db(), request); } diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp index 9e9d6e07824..acc5fca996a 100644 --- a/src/mongo/db/repl/storage_interface_impl.cpp +++ b/src/mongo/db/repl/storage_interface_impl.cpp @@ -67,7 +67,6 @@ #include "mongo/db/operation_context.h" #include "mongo/db/ops/delete_request.h" #include "mongo/db/ops/parsed_update.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/ops/update_request.h" #include "mongo/db/query/get_executor.h" #include "mongo/db/query/internal_plans.h" @@ -881,10 +880,8 @@ Status StorageInterfaceImpl::upsertById(OperationContext* opCtx, // We can create an UpdateRequest now that the collection's namespace has been resolved, in // the event it was specified as a UUID. UpdateRequest request(collection->ns()); - UpdateLifecycleImpl lifeCycle(collection->ns()); request.setQuery(query); request.setUpdates(update); - request.setLifecycle(&lifeCycle); request.setUpsert(true); invariant(!request.isMulti()); // This follows from using an exact _id query. invariant(!request.shouldReturnAnyDocs()); @@ -923,10 +920,8 @@ Status StorageInterfaceImpl::putSingleton(OperationContext* opCtx, const NamespaceString& nss, const TimestampedBSONObj& update) { UpdateRequest request(nss); - UpdateLifecycleImpl lifeCycle(nss); request.setQuery({}); request.setUpdates(update.obj); - request.setLifecycle(&lifeCycle); request.setUpsert(true); return _updateWithQuery(opCtx, request, update.timestamp); } @@ -936,10 +931,8 @@ Status StorageInterfaceImpl::updateSingleton(OperationContext* opCtx, const BSONObj& query, const TimestampedBSONObj& update) { UpdateRequest request(nss); - UpdateLifecycleImpl lifeCycle(nss); request.setQuery(query); request.setUpdates(update.obj); - request.setLifecycle(&lifeCycle); invariant(!request.isUpsert()); return _updateWithQuery(opCtx, request, update.timestamp); } diff --git a/src/mongo/db/s/sharding_initialization_mongod.cpp b/src/mongo/db/s/sharding_initialization_mongod.cpp index f42c2d54e09..b24cab09a4c 100644 --- a/src/mongo/db/s/sharding_initialization_mongod.cpp +++ b/src/mongo/db/s/sharding_initialization_mongod.cpp @@ -47,7 +47,6 @@ #include "mongo/db/logical_time_validator.h" #include "mongo/db/operation_context.h" #include "mongo/db/ops/update.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/repl/replication_coordinator.h" #include "mongo/db/s/chunk_splitter.h" #include "mongo/db/s/periodic_balancer_config_refresher.h" @@ -310,8 +309,6 @@ Status ShardingInitializationMongoD::updateShardIdentityConfigString( UpdateRequest updateReq(NamespaceString::kServerConfigurationNamespace); updateReq.setQuery(BSON("_id" << ShardIdentityType::IdName)); updateReq.setUpdates(updateObj); - UpdateLifecycleImpl updateLifecycle(NamespaceString::kServerConfigurationNamespace); - updateReq.setLifecycle(&updateLifecycle); try { AutoGetOrCreateDb autoDb( diff --git a/src/mongo/db/s/sharding_state_recovery.cpp b/src/mongo/db/s/sharding_state_recovery.cpp index 1ae9d9bca32..6fc39417200 100644 --- a/src/mongo/db/s/sharding_state_recovery.cpp +++ b/src/mongo/db/s/sharding_state_recovery.cpp @@ -43,7 +43,6 @@ #include "mongo/db/namespace_string.h" #include "mongo/db/operation_context.h" #include "mongo/db/ops/update.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/ops/update_request.h" #include "mongo/db/repl/bson_extract_optime.h" #include "mongo/db/repl/optime.h" @@ -174,8 +173,6 @@ Status modifyRecoveryDocument(OperationContext* opCtx, updateReq.setQuery(RecoveryDocument::getQuery()); updateReq.setUpdates(updateObj); updateReq.setUpsert(); - UpdateLifecycleImpl updateLifecycle(NamespaceString::kServerConfigurationNamespace); - updateReq.setLifecycle(&updateLifecycle); UpdateResult result = update(opCtx, autoGetOrCreateDb->getDb(), updateReq); invariant(result.numDocsModified == 1 || !result.upserted.isEmpty()); diff --git a/src/mongo/dbtests/query_stage_delete.cpp b/src/mongo/dbtests/query_stage_delete.cpp index e22ee615b3f..5030867e525 100644 --- a/src/mongo/dbtests/query_stage_delete.cpp +++ b/src/mongo/dbtests/query_stage_delete.cpp @@ -131,6 +131,7 @@ public: dbtests::WriteContextForTests ctx(&_opCtx, nss.ns()); Collection* coll = ctx.getCollection(); + ASSERT(coll); // Get the RecordIds that would be returned by an in-order scan. vector<RecordId> recordIds; @@ -163,11 +164,11 @@ public: } // Remove recordIds[targetDocIndex]; - deleteStage.saveState(); + static_cast<PlanStage*>(&deleteStage)->saveState(); BSONObj targetDoc = coll->docFor(&_opCtx, recordIds[targetDocIndex]).value(); ASSERT(!targetDoc.isEmpty()); remove(targetDoc); - deleteStage.restoreState(); + static_cast<PlanStage*>(&deleteStage)->restoreState(); // Remove the rest. while (!deleteStage.isEOF()) { @@ -190,6 +191,7 @@ public: // Various variables we'll need. dbtests::WriteContextForTests ctx(&_opCtx, nss.ns()); Collection* coll = ctx.getCollection(); + ASSERT(coll); const int targetDocIndex = 0; const BSONObj query = BSON("foo" << BSON("$gte" << targetDocIndex)); const auto ws = make_unique<WorkingSet>(); diff --git a/src/mongo/dbtests/query_stage_update.cpp b/src/mongo/dbtests/query_stage_update.cpp index bc07c4ec803..d1f0676c92b 100644 --- a/src/mongo/dbtests/query_stage_update.cpp +++ b/src/mongo/dbtests/query_stage_update.cpp @@ -48,7 +48,6 @@ #include "mongo/db/jsobj.h" #include "mongo/db/json.h" #include "mongo/db/namespace_string.h" -#include "mongo/db/ops/update_lifecycle_impl.h" #include "mongo/db/ops/update_request.h" #include "mongo/db/query/canonical_query.h" #include "mongo/db/service_context.h" @@ -201,13 +200,12 @@ public: const CollatorInterface* collator = nullptr; UpdateDriver driver(new ExpressionContext(&_opCtx, collator)); Collection* collection = ctx.getCollection(); + ASSERT(collection); // Collection should be empty. ASSERT_EQUALS(0U, count(BSONObj())); UpdateRequest request(nss); - UpdateLifecycleImpl updateLifecycle(nss); - request.setLifecycle(&updateLifecycle); // Update is the upsert {_id: 0, x: 1}, {$set: {y: 2}}. BSONObj query = fromjson("{_id: 0, x: 1}"); @@ -273,14 +271,13 @@ public: UpdateDriver driver(new ExpressionContext(&_opCtx, collator)); Database* db = ctx.db(); Collection* coll = db->getCollection(&_opCtx, nss); + ASSERT(coll); // Get the RecordIds that would be returned by an in-order scan. vector<RecordId> recordIds; getRecordIds(coll, CollectionScanParams::FORWARD, &recordIds); UpdateRequest request(nss); - UpdateLifecycleImpl updateLifecycle(nss); - request.setLifecycle(&updateLifecycle); // Update is a multi-update that sets 'bar' to 3 in every document // where foo is less than 5. @@ -325,11 +322,11 @@ public: } // Remove recordIds[targetDocIndex]; - updateStage->saveState(); + static_cast<PlanStage*>(updateStage.get())->saveState(); BSONObj targetDoc = coll->docFor(&_opCtx, recordIds[targetDocIndex]).value(); ASSERT(!targetDoc.isEmpty()); remove(targetDoc); - updateStage->restoreState(); + static_cast<PlanStage*>(updateStage.get())->restoreState(); // Do the remaining updates. while (!updateStage->isEOF()) { @@ -382,7 +379,7 @@ public: dbtests::WriteContextForTests ctx(&_opCtx, nss.ns()); OpDebug* opDebug = &CurOp::get(_opCtx)->debug(); Collection* coll = ctx.getCollection(); - UpdateLifecycleImpl updateLifecycle(nss); + ASSERT(coll); UpdateRequest request(nss); const CollatorInterface* collator = nullptr; UpdateDriver driver(new ExpressionContext(&_opCtx, collator)); @@ -401,7 +398,6 @@ public: request.setSort(BSONObj()); request.setMulti(false); request.setReturnDocs(UpdateRequest::RETURN_OLD); - request.setLifecycle(&updateLifecycle); const std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; @@ -473,7 +469,7 @@ public: dbtests::WriteContextForTests ctx(&_opCtx, nss.ns()); OpDebug* opDebug = &CurOp::get(_opCtx)->debug(); Collection* coll = ctx.getCollection(); - UpdateLifecycleImpl updateLifecycle(nss); + ASSERT(coll); UpdateRequest request(nss); const CollatorInterface* collator = nullptr; UpdateDriver driver(new ExpressionContext(&_opCtx, collator)); @@ -492,7 +488,6 @@ public: request.setSort(BSONObj()); request.setMulti(false); request.setReturnDocs(UpdateRequest::RETURN_NEW); - request.setLifecycle(&updateLifecycle); const std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters; |