diff options
-rw-r--r-- | src/mongo/db/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/exec/delete.cpp | 40 | ||||
-rw-r--r-- | src/mongo/db/exec/delete.h | 14 | ||||
-rw-r--r-- | src/mongo/db/exec/stagedebug_cmd.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/query/get_executor.cpp | 23 | ||||
-rw-r--r-- | src/mongo/db/query/internal_plans.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/query/internal_plans.h | 6 | ||||
-rw-r--r-- | src/mongo/db/repl/storage_interface_impl.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/s/collection_range_deleter.cpp | 58 | ||||
-rw-r--r-- | src/mongo/db/storage/remove_saver.h | 5 | ||||
-rw-r--r-- | src/mongo/db/ttl.cpp | 8 | ||||
-rw-r--r-- | src/mongo/dbtests/query_stage_delete.cpp | 16 |
12 files changed, 104 insertions, 91 deletions
diff --git a/src/mongo/db/SConscript b/src/mongo/db/SConscript index 5e5d85fb2e3..5e2b6201f06 100644 --- a/src/mongo/db/SConscript +++ b/src/mongo/db/SConscript @@ -1186,6 +1186,7 @@ env.Library( 'stats/serveronly_stats', 'storage/oplog_hack', 'storage/storage_options', + 'storage/remove_saver', 'update/update_driver', ], LIBDEPS_PRIVATE=[ diff --git a/src/mongo/db/exec/delete.cpp b/src/mongo/db/exec/delete.cpp index b1ae8b66942..71e6f6a5628 100644 --- a/src/mongo/db/exec/delete.cpp +++ b/src/mongo/db/exec/delete.cpp @@ -60,12 +60,12 @@ namespace { * Returns true if we should throw a WriteConflictException in order to retry the operation in * the case of a conflict. Returns false if we should skip the document and keep going. */ -bool shouldRestartDeleteIfNoLongerMatches(const DeleteStageParams& params) { +bool shouldRestartDeleteIfNoLongerMatches(const DeleteStageParams* params) { // When we're doing a findAndModify with a sort, the sort will have a limit of 1, so it will not // produce any more results even if there is another matching document. Throw a WCE here so that // these operations get another chance to find a matching document. The findAndModify command // should automatically retry if it gets a WCE. - return params.returnDeleted && !params.sort.isEmpty(); + return params->returnDeleted && !params->sort.isEmpty(); }; } // namespace @@ -74,12 +74,12 @@ bool shouldRestartDeleteIfNoLongerMatches(const DeleteStageParams& params) { const char* DeleteStage::kStageType = "DELETE"; DeleteStage::DeleteStage(OperationContext* opCtx, - const DeleteStageParams& params, + std::unique_ptr<DeleteStageParams> params, WorkingSet* ws, Collection* collection, PlanStage* child) : RequiresMutableCollectionStage(kStageType, opCtx, collection), - _params(params), + _params(std::move(params)), _ws(ws), _idRetrying(WorkingSet::INVALID_ID), _idReturning(WorkingSet::INVALID_ID) { @@ -87,7 +87,7 @@ DeleteStage::DeleteStage(OperationContext* opCtx, } bool DeleteStage::isEOF() { - if (!_params.isMulti && _specificStats.docsDeleted > 0) { + if (!_params->isMulti && _specificStats.docsDeleted > 0) { return true; } return _idRetrying == WorkingSet::INVALID_ID && _idReturning == WorkingSet::INVALID_ID && @@ -103,7 +103,7 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { // and prevented us from returning ADVANCED with the old version of the document. if (_idReturning != WorkingSet::INVALID_ID) { // We should only get here if we were trying to return something before. - invariant(_params.returnDeleted); + invariant(_params->returnDeleted); WorkingSetMember* member = _ws->get(_idReturning); invariant(member->getState() == WorkingSetMember::OWNED_OBJ); @@ -164,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(); @@ -174,7 +174,7 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { if (!docStillMatches) { // Either the document has already been deleted, or it has been updated such that it no // longer matches the predicate. - if (shouldRestartDeleteIfNoLongerMatches(_params)) { + if (shouldRestartDeleteIfNoLongerMatches(_params.get())) { throw WriteConflictException(); } return PlanStage::NEED_TIME; @@ -182,13 +182,17 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { // Ensure that the BSONObj underlying the WorkingSetMember is owned because saveState() is // allowed to free the memory. - if (_params.returnDeleted) { + if (_params->returnDeleted) { // Save a copy of the document that is about to get deleted, but keep it in the RID_AND_OBJ // state in case we need to retry deleting it. BSONObj deletedDoc = member->obj.value(); member->obj.setValue(deletedDoc.getOwned()); } + if (_params->removeSaver) { + uassertStatusOK(_params->removeSaver->goingToDelete(member->obj.value())); + } + // TODO: Do we want to buffer docs and delete them in a group rather than saving/restoring state // repeatedly? @@ -200,17 +204,17 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { } // Do the write, unless this is an explain. - if (!_params.isExplain) { + if (!_params->isExplain) { try { WriteUnitOfWork wunit(getOpCtx()); collection()->deleteDocument(getOpCtx(), - _params.stmtId, + _params->stmtId, recordId, - _params.opDebug, - _params.fromMigrate, + _params->opDebug, + _params->fromMigrate, false, - _params.returnDeleted ? Collection::StoreDeletedDoc::On - : Collection::StoreDeletedDoc::Off); + _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. @@ -219,7 +223,7 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { } ++_specificStats.docsDeleted; - if (_params.returnDeleted) { + if (_params->returnDeleted) { // After deleting the document, the RecordId associated with this member is invalid. // Remove the 'recordId' from the WorkingSetMember before returning it. member->recordId = RecordId(); @@ -234,7 +238,7 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { } catch (const WriteConflictException&) { // Note we don't need to retry anything in this case since the delete already was committed. // However, we still need to return the deleted document (if it was requested). - if (_params.returnDeleted) { + if (_params->returnDeleted) { // member->obj should refer to the deleted document. invariant(member->getState() == WorkingSetMember::OWNED_OBJ); @@ -246,7 +250,7 @@ PlanStage::StageState DeleteStage::doWork(WorkingSetID* out) { return NEED_YIELD; } - if (_params.returnDeleted) { + if (_params->returnDeleted) { // member->obj should refer to the deleted document. invariant(member->getState() == WorkingSetMember::OWNED_OBJ); diff --git a/src/mongo/db/exec/delete.h b/src/mongo/db/exec/delete.h index 4fe3590745d..198981b8e64 100644 --- a/src/mongo/db/exec/delete.h +++ b/src/mongo/db/exec/delete.h @@ -33,6 +33,7 @@ #include "mongo/db/exec/requires_collection_stage.h" #include "mongo/db/jsobj.h" #include "mongo/db/logical_session_id.h" +#include "mongo/db/storage/remove_saver.h" namespace mongo { @@ -75,6 +76,15 @@ struct DeleteStageParams { // Optional. When not null, delete metrics are recorded here. OpDebug* opDebug; + + // Optional. When not null, send document about to be deleted to removeSaver. + // RemoveSaver is called before actual deletes are executed. + // Note: the differentiating factor between this and returnDeleted is that the caller will get + // the deleted document after it was already deleted. That means that if the caller would have + // to use the removeSaver at that point, they miss the document if the process dies before it + // reaches the removeSaver. However, this is still best effort since the RemoveSaver + // operates on a different persistence system from the the database storage engine. + std::unique_ptr<RemoveSaver> removeSaver; }; /** @@ -90,7 +100,7 @@ class DeleteStage final : public RequiresMutableCollectionStage { public: DeleteStage(OperationContext* opCtx, - const DeleteStageParams& params, + std::unique_ptr<DeleteStageParams> params, WorkingSet* ws, Collection* collection, PlanStage* child); @@ -127,7 +137,7 @@ private: */ StageState prepareToRetryWSM(WorkingSetID idToRetry, WorkingSetID* out); - DeleteStageParams _params; + std::unique_ptr<DeleteStageParams> _params; // Not owned by us. WorkingSet* _ws; diff --git a/src/mongo/db/exec/stagedebug_cmd.cpp b/src/mongo/db/exec/stagedebug_cmd.cpp index 2a9aa368cf3..12a5dd29745 100644 --- a/src/mongo/db/exec/stagedebug_cmd.cpp +++ b/src/mongo/db/exec/stagedebug_cmd.cpp @@ -495,9 +495,9 @@ public: uassert(28734, "Can't parse sub-node of DELETE: " + nodeArgs["node"].Obj().toString(), NULL != subNode); - DeleteStageParams params; - params.isMulti = nodeArgs["isMulti"].Bool(); - return new DeleteStage(opCtx, params, workingSet, collection, subNode); + auto params = std::make_unique<DeleteStageParams>(); + params->isMulti = nodeArgs["isMulti"].Bool(); + return new DeleteStage(opCtx, std::move(params), workingSet, collection, subNode); } else { return NULL; } diff --git a/src/mongo/db/query/get_executor.cpp b/src/mongo/db/query/get_executor.cpp index 2bb817d1699..c3bdcae3786 100644 --- a/src/mongo/db/query/get_executor.cpp +++ b/src/mongo/db/query/get_executor.cpp @@ -830,14 +830,14 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDelete( str::stream() << "Not primary while removing from " << nss.ns()); } - DeleteStageParams deleteStageParams; - deleteStageParams.isMulti = request->isMulti(); - deleteStageParams.fromMigrate = request->isFromMigrate(); - deleteStageParams.isExplain = request->isExplain(); - deleteStageParams.returnDeleted = request->shouldReturnDeleted(); - deleteStageParams.sort = request->getSort(); - deleteStageParams.opDebug = opDebug; - deleteStageParams.stmtId = request->getStmtId(); + auto deleteStageParams = std::make_unique<DeleteStageParams>(); + deleteStageParams->isMulti = request->isMulti(); + deleteStageParams->fromMigrate = request->isFromMigrate(); + deleteStageParams->isExplain = request->isExplain(); + deleteStageParams->returnDeleted = request->shouldReturnDeleted(); + deleteStageParams->sort = request->getSort(); + deleteStageParams->opDebug = opDebug; + deleteStageParams->stmtId = request->getStmtId(); unique_ptr<WorkingSet> ws = make_unique<WorkingSet>(); const PlanExecutor::YieldPolicy policy = parsedDelete->yieldPolicy(); @@ -878,7 +878,7 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDelete( auto idHackStage = std::make_unique<IDHackStage>( opCtx, unparsedQuery["_id"].wrap(), ws.get(), descriptor); unique_ptr<DeleteStage> root = make_unique<DeleteStage>( - opCtx, deleteStageParams, ws.get(), collection, idHackStage.release()); + opCtx, std::move(deleteStageParams), ws.get(), collection, idHackStage.release()); return PlanExecutor::make(opCtx, std::move(ws), std::move(root), collection, policy); } @@ -903,10 +903,11 @@ StatusWith<unique_ptr<PlanExecutor, PlanExecutor::Deleter>> getExecutorDelete( unique_ptr<QuerySolution> querySolution = std::move(executionResult.getValue().querySolution); unique_ptr<PlanStage> root = std::move(executionResult.getValue().root); - deleteStageParams.canonicalQuery = cq.get(); + deleteStageParams->canonicalQuery = cq.get(); invariant(root); - root = make_unique<DeleteStage>(opCtx, deleteStageParams, ws.get(), collection, root.release()); + root = make_unique<DeleteStage>( + opCtx, std::move(deleteStageParams), ws.get(), collection, root.release()); if (!request->getProj().isEmpty()) { invariant(request->shouldReturnDeleted()); diff --git a/src/mongo/db/query/internal_plans.cpp b/src/mongo/db/query/internal_plans.cpp index cc29c363412..e2e767d567b 100644 --- a/src/mongo/db/query/internal_plans.cpp +++ b/src/mongo/db/query/internal_plans.cpp @@ -78,7 +78,7 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> InternalPlanner::collection std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> InternalPlanner::deleteWithCollectionScan( OperationContext* opCtx, Collection* collection, - const DeleteStageParams& params, + std::unique_ptr<DeleteStageParams> params, PlanExecutor::YieldPolicy yieldPolicy, Direction direction, const RecordId& startLoc) { @@ -87,7 +87,8 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> InternalPlanner::deleteWith auto root = _collectionScan(opCtx, ws.get(), collection, direction, startLoc); - root = stdx::make_unique<DeleteStage>(opCtx, params, ws.get(), collection, root.release()); + root = stdx::make_unique<DeleteStage>( + opCtx, std::move(params), ws.get(), collection, root.release()); auto executor = PlanExecutor::make(opCtx, std::move(ws), std::move(root), collection, yieldPolicy); @@ -127,7 +128,7 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> InternalPlanner::indexScan( std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> InternalPlanner::deleteWithIndexScan( OperationContext* opCtx, Collection* collection, - const DeleteStageParams& params, + std::unique_ptr<DeleteStageParams> params, const IndexDescriptor* descriptor, const BSONObj& startKey, const BSONObj& endKey, @@ -147,7 +148,8 @@ std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> InternalPlanner::deleteWith direction, InternalPlanner::IXSCAN_FETCH); - root = stdx::make_unique<DeleteStage>(opCtx, params, ws.get(), collection, root.release()); + root = stdx::make_unique<DeleteStage>( + opCtx, std::move(params), ws.get(), collection, root.release()); auto executor = PlanExecutor::make(opCtx, std::move(ws), std::move(root), collection, yieldPolicy); diff --git a/src/mongo/db/query/internal_plans.h b/src/mongo/db/query/internal_plans.h index c12fd3e3f5c..a33beb1a963 100644 --- a/src/mongo/db/query/internal_plans.h +++ b/src/mongo/db/query/internal_plans.h @@ -31,6 +31,7 @@ #pragma once #include "mongo/base/string_data.h" +#include "mongo/db/exec/delete.h" #include "mongo/db/query/plan_executor.h" #include "mongo/db/record_id.h" @@ -42,7 +43,6 @@ class IndexDescriptor; class OperationContext; class PlanStage; class WorkingSet; -struct DeleteStageParams; struct UpdateStageParams; /** @@ -83,7 +83,7 @@ public: static std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> deleteWithCollectionScan( OperationContext* opCtx, Collection* collection, - const DeleteStageParams& params, + std::unique_ptr<DeleteStageParams> params, PlanExecutor::YieldPolicy yieldPolicy, Direction direction = FORWARD, const RecordId& startLoc = RecordId()); @@ -108,7 +108,7 @@ public: static std::unique_ptr<PlanExecutor, PlanExecutor::Deleter> deleteWithIndexScan( OperationContext* opCtx, Collection* collection, - const DeleteStageParams& params, + std::unique_ptr<DeleteStageParams> params, const IndexDescriptor* descriptor, const BSONObj& startKey, const BSONObj& endKey, diff --git a/src/mongo/db/repl/storage_interface_impl.cpp b/src/mongo/db/repl/storage_interface_impl.cpp index f0b0ecbf467..a5961b8d4ca 100644 --- a/src/mongo/db/repl/storage_interface_impl.cpp +++ b/src/mongo/db/repl/storage_interface_impl.cpp @@ -568,10 +568,10 @@ namespace { /** * Returns DeleteStageParams for deleteOne with fetch. */ -DeleteStageParams makeDeleteStageParamsForDeleteDocuments() { - DeleteStageParams deleteStageParams; - deleteStageParams.isMulti = true; - deleteStageParams.returnDeleted = true; +std::unique_ptr<DeleteStageParams> makeDeleteStageParamsForDeleteDocuments() { + auto deleteStageParams = std::make_unique<DeleteStageParams>(); + deleteStageParams->isMulti = true; + deleteStageParams->returnDeleted = true; return deleteStageParams; } diff --git a/src/mongo/db/s/collection_range_deleter.cpp b/src/mongo/db/s/collection_range_deleter.cpp index ead8f441758..4ba855355e8 100644 --- a/src/mongo/db/s/collection_range_deleter.cpp +++ b/src/mongo/db/s/collection_range_deleter.cpp @@ -42,11 +42,13 @@ #include "mongo/db/client.h" #include "mongo/db/concurrency/write_conflict_exception.h" #include "mongo/db/dbhelpers.h" +#include "mongo/db/exec/delete.h" #include "mongo/db/exec/working_set_common.h" #include "mongo/db/index/index_descriptor.h" #include "mongo/db/keypattern.h" #include "mongo/db/operation_context.h" #include "mongo/db/query/internal_plans.h" +#include "mongo/db/query/plan_yield_policy.h" #include "mongo/db/query/query_knobs.h" #include "mongo/db/query/query_planner.h" #include "mongo/db/repl/repl_client_info.h" @@ -387,56 +389,46 @@ StatusWith<int> CollectionRangeDeleter::_doDeletion(OperationContext* opCtx, return {ErrorCodes::InternalError, msg}; } - boost::optional<RemoveSaver> saver; + auto deleteStageParams = std::make_unique<DeleteStageParams>(); + deleteStageParams->fromMigrate = true; + deleteStageParams->isMulti = true; + deleteStageParams->returnDeleted = true; + if (serverGlobalParams.moveParanoia) { - saver.emplace("moveChunk", nss.ns(), "cleaning"); + deleteStageParams->removeSaver = + std::make_unique<RemoveSaver>("moveChunk", nss.ns(), "cleaning"); } - auto halfOpen = BoundInclusion::kIncludeStartKeyOnly; - auto manual = PlanExecutor::YIELD_MANUAL; - auto forward = InternalPlanner::FORWARD; - auto fetch = InternalPlanner::IXSCAN_FETCH; + auto exec = InternalPlanner::deleteWithIndexScan(opCtx, + collection, + std::move(deleteStageParams), + descriptor, + min, + max, + BoundInclusion::kIncludeStartKeyOnly, + PlanExecutor::YIELD_MANUAL, + InternalPlanner::FORWARD); - auto exec = InternalPlanner::indexScan( - opCtx, collection, descriptor, min, max, halfOpen, manual, forward, fetch); + PlanYieldPolicy planYieldPolicy(exec.get(), PlanExecutor::YIELD_MANUAL); int numDeleted = 0; do { - RecordId rloc; - BSONObj obj; - PlanExecutor::ExecState state = exec->getNext(&obj, &rloc); + BSONObj deletedObj; + PlanExecutor::ExecState state = exec->getNext(&deletedObj, nullptr); + if (state == PlanExecutor::IS_EOF) { break; } + if (state == PlanExecutor::FAILURE || state == PlanExecutor::DEAD) { warning() << PlanExecutor::statestr(state) << " - cursor error while trying to delete " << redact(min) << " to " << redact(max) << " in " << nss << ": " - << redact(WorkingSetCommon::toStatusString(obj)) + << redact(WorkingSetCommon::toStatusString(deletedObj)) << ", stats: " << Explain::getWinningPlanStats(exec.get()); break; } - invariant(PlanExecutor::ADVANCED == state); - - exec->saveState(); - writeConflictRetry(opCtx, "delete range", nss.ns(), [&] { - WriteUnitOfWork wuow(opCtx); - if (saver) { - uassertStatusOK(saver->goingToDelete(obj)); - } - collection->deleteDocument(opCtx, kUninitializedStmtId, rloc, nullptr, true); - wuow.commit(); - }); - - try { - exec->restoreState(); - } catch (const DBException& ex) { - warning() << "error restoring cursor state while trying to delete " << redact(min) - << " to " << redact(max) << " in " << nss - << ", stats: " << Explain::getWinningPlanStats(exec.get()) << ": " - << redact(ex.toStatus()); - break; - } + invariant(PlanExecutor::ADVANCED == state); ShardingStatistics::get(opCtx).countDocsDeletedOnDonor.addAndFetch(1); } while (++numDeleted < maxToDelete); diff --git a/src/mongo/db/storage/remove_saver.h b/src/mongo/db/storage/remove_saver.h index 78057622444..dbde3f728c6 100644 --- a/src/mongo/db/storage/remove_saver.h +++ b/src/mongo/db/storage/remove_saver.h @@ -43,7 +43,10 @@ namespace mongo { /** - * for saving deleted bson objects to a flat file + * This class provides facility for saving bson objects to a flat file. The common use case is for + * making a back-up copy of a document before an internal operation (like migration or rollback) + * deletes it. To use this correctly, the caller must call goingToDelete first before the actual + * deletion, otherwise the document will be lost if the process gets terminated in between. */ class RemoveSaver { MONGO_DISALLOW_COPYING(RemoveSaver); diff --git a/src/mongo/db/ttl.cpp b/src/mongo/db/ttl.cpp index 0ad9cb0af35..9f4f4eeb91d 100644 --- a/src/mongo/db/ttl.cpp +++ b/src/mongo/db/ttl.cpp @@ -253,14 +253,14 @@ private: auto canonicalQuery = CanonicalQuery::canonicalize(opCtx, std::move(qr)); invariant(canonicalQuery.getStatus()); - DeleteStageParams params; - params.isMulti = true; - params.canonicalQuery = canonicalQuery.getValue().get(); + auto params = std::make_unique<DeleteStageParams>(); + params->isMulti = true; + params->canonicalQuery = canonicalQuery.getValue().get(); auto exec = InternalPlanner::deleteWithIndexScan(opCtx, collection, - params, + std::move(params), desc, startKey, endKey, diff --git a/src/mongo/dbtests/query_stage_delete.cpp b/src/mongo/dbtests/query_stage_delete.cpp index 5030867e525..4b29586b12f 100644 --- a/src/mongo/dbtests/query_stage_delete.cpp +++ b/src/mongo/dbtests/query_stage_delete.cpp @@ -143,12 +143,12 @@ public: collScanParams.tailable = false; // Configure the delete stage. - DeleteStageParams deleteStageParams; - deleteStageParams.isMulti = true; + auto deleteStageParams = std::make_unique<DeleteStageParams>(); + deleteStageParams->isMulti = true; WorkingSet ws; DeleteStage deleteStage(&_opCtx, - deleteStageParams, + std::move(deleteStageParams), &ws, coll, new CollectionScan(&_opCtx, coll, collScanParams, &ws, NULL)); @@ -213,12 +213,12 @@ public: qds->pushBack(id); // Configure the delete. - DeleteStageParams deleteParams; - deleteParams.returnDeleted = true; - deleteParams.canonicalQuery = cq.get(); + auto deleteParams = std::make_unique<DeleteStageParams>(); + deleteParams->returnDeleted = true; + deleteParams->canonicalQuery = cq.get(); - const auto deleteStage = - make_unique<DeleteStage>(&_opCtx, deleteParams, ws.get(), coll, qds.release()); + const auto deleteStage = make_unique<DeleteStage>( + &_opCtx, std::move(deleteParams), ws.get(), coll, qds.release()); const DeleteStats* stats = static_cast<const DeleteStats*>(deleteStage->getSpecificStats()); |