diff options
author | Andy Schwerin <schwerin@mongodb.com> | 2015-06-01 17:32:33 -0400 |
---|---|---|
committer | Andy Schwerin <schwerin@mongodb.com> | 2015-06-02 12:05:59 -0400 |
commit | 754f482c204160bf0c74373b64ba3406604f0731 (patch) | |
tree | 479fe9e9afd65bf78ae30b73d00649d41d67389b /src | |
parent | 908d0cedf82b50c93932916685c4f8fa4748cc6f (diff) | |
download | mongo-754f482c204160bf0c74373b64ba3406604f0731.tar.gz |
SERVER-14995 Move _killPending from CurOp to OperationContext.
Also, limit the lifetime of OperationContext in MongoD so that it goes out of
scope before sending a reply to the client. This is necessary so that
operations do not appear in the currentOp command result after the server
sends a response to the client.
Diffstat (limited to 'src')
-rw-r--r-- | src/mongo/db/clientlistplugin.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/commands/current_op.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/curop.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/curop.h | 9 | ||||
-rw-r--r-- | src/mongo/db/db.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/operation_context.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/operation_context.h | 33 | ||||
-rw-r--r-- | src/mongo/db/operation_context_impl.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/operation_context_impl.h | 4 | ||||
-rw-r--r-- | src/mongo/db/operation_context_noop.h | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/operation_context_repl_mock.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/operation_context_repl_mock.h | 4 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator.h | 6 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_impl.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_impl.h | 8 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_mock.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_coordinator_mock.h | 6 | ||||
-rw-r--r-- | src/mongo/db/service_context_d.cpp | 6 | ||||
-rw-r--r-- | src/mongo/s/d_migrate.cpp | 2 |
19 files changed, 84 insertions, 65 deletions
diff --git a/src/mongo/db/clientlistplugin.cpp b/src/mongo/db/clientlistplugin.cpp index 6eb854c739d..42499f5c116 100644 --- a/src/mongo/db/clientlistplugin.cpp +++ b/src/mongo/db/clientlistplugin.cpp @@ -208,12 +208,12 @@ namespace { b.appendBool("active", static_cast<bool>(txn)); if (txn) { b.append("opid", txn->getOpID()); - - // CurOp - if (CurOp::get(txn)) { - CurOp::get(txn)->reportState(&b); + if (txn->isKillPending()) { + b.append("killPending", true); } + CurOp::get(txn)->reportState(&b); + // LockState if (txn->lockState()) { StringBuilder ss; diff --git a/src/mongo/db/commands/current_op.cpp b/src/mongo/db/commands/current_op.cpp index b26c811d2d8..d28cb91874e 100644 --- a/src/mongo/db/commands/current_op.cpp +++ b/src/mongo/db/commands/current_op.cpp @@ -124,11 +124,12 @@ namespace mongo { infoBuilder.appendBool("active", static_cast<bool>(opCtx)); if (opCtx) { infoBuilder.append("opid", opCtx->getOpID()); - // CurOp - if (CurOp::get(opCtx)) { - CurOp::get(opCtx)->reportState(&infoBuilder); + if (opCtx->isKillPending()) { + infoBuilder.append("killPending", true); } + CurOp::get(opCtx)->reportState(&infoBuilder); + // LockState Locker::LockerInfo lockerInfo; opCtx->lockState()->getLockerInfo(&lockerInfo); diff --git a/src/mongo/db/curop.cpp b/src/mongo/db/curop.cpp index 4c41b5bd365..f896b1524b3 100644 --- a/src/mongo/db/curop.cpp +++ b/src/mongo/db/curop.cpp @@ -165,7 +165,6 @@ namespace mongo { _maxTimeTracker.reset(); _message = ""; _progressMeter.finished(); - _killPending.store(0); _numYields = 0; _expectedLatencyMs = 0; _op = 0; @@ -268,16 +267,9 @@ namespace mongo { } } - if( killPending() ) - builder->append("killPending", true); - builder->append( "numYields" , _numYields ); } - void CurOp::kill() { - _killPending.store(1); - } - void CurOp::setMaxTimeMicros(uint64_t maxTimeMicros) { _maxTimeMicros = maxTimeMicros; diff --git a/src/mongo/db/curop.h b/src/mongo/db/curop.h index f58ee576370..03467cd0cb9 100644 --- a/src/mongo/db/curop.h +++ b/src/mongo/db/curop.h @@ -242,9 +242,6 @@ namespace mongo { /** * Checks whether this operation has been running longer than its time limit. Returns * false if not, or if the operation has no time limit. - * - * Note that KillCurrentOp objects are responsible for interrupting CurOp objects that - * have exceeded their allotted time; CurOp objects do not interrupt themselves. */ bool maxTimeHasExpired(); @@ -298,12 +295,9 @@ namespace mongo { std::string getMessage() const { return _message.toString(); } ProgressMeter& getProgressMeter() { return _progressMeter; } CurOp *parent() const { return _parent; } - void kill(); - bool killPendingStrict() const { return _killPending.load(); } - bool killPending() const { return _killPending.loadRelaxed(); } void yielded() { _numYields++; } int numYields() const { return _numYields; } - + long long getExpectedLatencyMs() const { return _expectedLatencyMs; } void setExpectedLatencyMs( long long latency ) { _expectedLatencyMs = latency; } @@ -336,7 +330,6 @@ namespace mongo { OpDebug _debug; ThreadSafeString _message; ProgressMeter _progressMeter; - AtomicInt32 _killPending; int _numYields; // this is how much "extra" time a query might take diff --git a/src/mongo/db/db.cpp b/src/mongo/db/db.cpp index 61678070dcf..f430cceb675 100644 --- a/src/mongo/db/db.cpp +++ b/src/mongo/db/db.cpp @@ -163,9 +163,13 @@ namespace mongo { break; } - OperationContextImpl txn; DbResponse dbresponse; - assembleResponse(&txn, m, dbresponse, port->remote()); + { + OperationContextImpl txn; + assembleResponse(&txn, m, dbresponse, port->remote()); + // txn must go out of scope here so that the operation cannot show up in + // currentOp results after the response reaches the client. + } if ( dbresponse.response ) { port->reply(m, *dbresponse.response, dbresponse.responseTo); diff --git a/src/mongo/db/operation_context.cpp b/src/mongo/db/operation_context.cpp index 5c87bee7220..703eca38030 100644 --- a/src/mongo/db/operation_context.cpp +++ b/src/mongo/db/operation_context.cpp @@ -43,4 +43,12 @@ namespace mongo { return _client; } + void OperationContext::markKilled() { + _killPending.store(1); + } + + bool OperationContext::isKillPending() const { + return _killPending.loadRelaxed(); + } + } // namespace mongo diff --git a/src/mongo/db/operation_context.h b/src/mongo/db/operation_context.h index e3c28ef2c05..291fd3b6923 100644 --- a/src/mongo/db/operation_context.h +++ b/src/mongo/db/operation_context.h @@ -33,6 +33,7 @@ #include "mongo/db/storage/recovery_unit.h" #include "mongo/db/concurrency/d_concurrency.h" #include "mongo/db/write_concern_options.h" +#include "mongo/platform/atomic_word.h" #include "mongo/util/decorable.h" namespace mongo { @@ -106,12 +107,14 @@ namespace mongo { // --- operation level info? --- /** - * If the thread is not interrupted, returns Status::OK(), otherwise returns the cause - * for the interruption. The throw variant returns a user assertion corresponding to the - * interruption status. + * Raises a UserAssertion if this operation is in a killed state. */ - virtual void checkForInterrupt() const = 0; - virtual Status checkForInterruptNoAssert() const = 0; + virtual void checkForInterrupt() = 0; + + /** + * Returns Status::OK() unless this operation is in a killed state. + */ + virtual Status checkForInterruptNoAssert() = 0; /** * Delegates to CurOp, but is included here to break dependencies. @@ -167,6 +170,25 @@ namespace mongo { */ virtual bool writesAreReplicated() const = 0; + /** + * Marks this operation as killed. + * + * Subsequent calls to checkForInterrupt and checkForInterruptNoAssert by the thread + * executing the operation will indicate that the operation has been killed. + * + * May be called by any thread that has locked the Client owning this operation context, + * or by the thread executing on behalf of this operation context. + */ + void markKilled(); + + /** + * Returns true if markKilled has been called on this operation context. + * + * May be called by any thread that has locked the Client owning this operation context, + * or by the thread executing on behalf of this operation context. + */ + bool isKillPending() const; + protected: OperationContext(Client* client, unsigned int opId, @@ -183,6 +205,7 @@ namespace mongo { // safe to access _locker in the destructor of OperationContext. Locker* const _locker; + AtomicInt32 _killPending{0}; WriteConcernOptions _writeConcern; }; diff --git a/src/mongo/db/operation_context_impl.cpp b/src/mongo/db/operation_context_impl.cpp index 6eccefce706..3f048b598d2 100644 --- a/src/mongo/db/operation_context_impl.cpp +++ b/src/mongo/db/operation_context_impl.cpp @@ -175,7 +175,7 @@ namespace { } // namespace - void OperationContextImpl::checkForInterrupt() const { + void OperationContextImpl::checkForInterrupt() { // We cannot interrupt operation, while it's inside of a write unit of work, because logOp // cannot handle being iterrupted. if (lockState()->inAWriteUnitOfWork()) return; @@ -183,14 +183,14 @@ namespace { uassertStatusOK(checkForInterruptNoAssert()); } - Status OperationContextImpl::checkForInterruptNoAssert() const { + Status OperationContextImpl::checkForInterruptNoAssert() { if (getGlobalServiceContext()->getKillAllOperations()) { return Status(ErrorCodes::InterruptedAtShutdown, "interrupted at shutdown"); } CurOp* curOp = CurOp::get(this); if (curOp->maxTimeHasExpired()) { - curOp->kill(); + markKilled(); return Status(ErrorCodes::ExceededTimeLimit, "operation exceeded time limit"); } @@ -199,11 +199,11 @@ namespace { log() << "set pending kill on " << (curOp->parent() ? "nested" : "top-level") << " op " << getOpID() << ", for checkForInterruptFail"; - curOp->kill(); + markKilled(); } } - if (curOp->killPending()) { + if (isKillPending()) { return Status(ErrorCodes::Interrupted, "operation was interrupted"); } diff --git a/src/mongo/db/operation_context_impl.h b/src/mongo/db/operation_context_impl.h index 362455581fd..e2065a1ba84 100644 --- a/src/mongo/db/operation_context_impl.h +++ b/src/mongo/db/operation_context_impl.h @@ -56,8 +56,8 @@ namespace mongo { virtual uint64_t getRemainingMaxTimeMicros() const override; - virtual void checkForInterrupt() const override; - virtual Status checkForInterruptNoAssert() const override; + virtual void checkForInterrupt() override; + virtual Status checkForInterruptNoAssert() override; virtual bool isPrimaryFor( StringData ns ) override; diff --git a/src/mongo/db/operation_context_noop.h b/src/mongo/db/operation_context_noop.h index 254f553445b..cd73c42dee0 100644 --- a/src/mongo/db/operation_context_noop.h +++ b/src/mongo/db/operation_context_noop.h @@ -88,8 +88,8 @@ namespace mongo { return &_pm; } - virtual void checkForInterrupt() const override { } - virtual Status checkForInterruptNoAssert() const override { + virtual void checkForInterrupt() override { } + virtual Status checkForInterruptNoAssert() override { return Status::OK(); } diff --git a/src/mongo/db/repl/operation_context_repl_mock.cpp b/src/mongo/db/repl/operation_context_repl_mock.cpp index 10d2e69beca..78587bda2f3 100644 --- a/src/mongo/db/repl/operation_context_repl_mock.cpp +++ b/src/mongo/db/repl/operation_context_repl_mock.cpp @@ -51,11 +51,11 @@ namespace repl { OperationContextReplMock::~OperationContextReplMock() = default; - void OperationContextReplMock::checkForInterrupt() const { + void OperationContextReplMock::checkForInterrupt() { uassertStatusOK(checkForInterruptNoAssert()); } - Status OperationContextReplMock::checkForInterruptNoAssert() const { + Status OperationContextReplMock::checkForInterruptNoAssert() { if (!_checkForInterruptStatus.isOK()) { return _checkForInterruptStatus; } diff --git a/src/mongo/db/repl/operation_context_repl_mock.h b/src/mongo/db/repl/operation_context_repl_mock.h index 6c5e76b046f..c79580f1482 100644 --- a/src/mongo/db/repl/operation_context_repl_mock.h +++ b/src/mongo/db/repl/operation_context_repl_mock.h @@ -50,9 +50,9 @@ namespace repl { OperationContextReplMock(Client* client, unsigned int opNum); virtual ~OperationContextReplMock(); - virtual void checkForInterrupt() const override; + virtual void checkForInterrupt() override; - virtual Status checkForInterruptNoAssert() const override; + virtual Status checkForInterruptNoAssert() override; void setCheckForInterruptStatus(Status status); diff --git a/src/mongo/db/repl/replication_coordinator.h b/src/mongo/db/repl/replication_coordinator.h index 9ce8a4e3a1f..d76d0fa3299 100644 --- a/src/mongo/db/repl/replication_coordinator.h +++ b/src/mongo/db/repl/replication_coordinator.h @@ -181,7 +181,7 @@ namespace repl { * ErrorCodes::ShutdownInProgress if we are mid-shutdown * ErrorCodes::Interrupted if the operation was killed with killop() */ - virtual StatusAndDuration awaitReplication(const OperationContext* txn, + virtual StatusAndDuration awaitReplication(OperationContext* txn, const OpTime& opTime, const WriteConcernOptions& writeConcern) = 0; @@ -190,7 +190,7 @@ namespace repl { * performed on the client associated with "txn". */ virtual StatusAndDuration awaitReplicationOfLastOpForClient( - const OperationContext* txn, + OperationContext* txn, const WriteConcernOptions& writeConcern) = 0; /** @@ -298,7 +298,7 @@ namespace repl { * Note: getDuration() on the returned ReadAfterOpTimeResponse will only be valid if * its didWait() method returns true. */ - virtual ReadAfterOpTimeResponse waitUntilOpTime(const OperationContext* txn, + virtual ReadAfterOpTimeResponse waitUntilOpTime(OperationContext* txn, const ReadAfterOpTimeArgs& settings) = 0; /** diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index 79a47f52a7f..2cd4e8c2f1f 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -767,7 +767,7 @@ namespace { } ReadAfterOpTimeResponse ReplicationCoordinatorImpl::waitUntilOpTime( - const OperationContext* txn, + OperationContext* txn, const ReadAfterOpTimeArgs& settings) { const auto& ts = settings.getOpTime(); const auto& timeout = settings.getTimeout(); @@ -1035,7 +1035,7 @@ namespace { } ReplicationCoordinator::StatusAndDuration ReplicationCoordinatorImpl::awaitReplication( - const OperationContext* txn, + OperationContext* txn, const OpTime& opTime, const WriteConcernOptions& writeConcern) { Timer timer; @@ -1045,7 +1045,7 @@ namespace { ReplicationCoordinator::StatusAndDuration ReplicationCoordinatorImpl::awaitReplicationOfLastOpForClient( - const OperationContext* txn, + OperationContext* txn, const WriteConcernOptions& writeConcern) { Timer timer; boost::unique_lock<boost::mutex> lock(_mutex); @@ -1060,7 +1060,7 @@ namespace { ReplicationCoordinator::StatusAndDuration ReplicationCoordinatorImpl::_awaitReplication_inlock( const Timer* timer, boost::unique_lock<boost::mutex>* lock, - const OperationContext* txn, + OperationContext* txn, const OpTime& opTime, const WriteConcernOptions& writeConcern) { diff --git a/src/mongo/db/repl/replication_coordinator_impl.h b/src/mongo/db/repl/replication_coordinator_impl.h index c015fdbfb97..d8dabc082eb 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.h +++ b/src/mongo/db/repl/replication_coordinator_impl.h @@ -123,12 +123,12 @@ namespace repl { virtual void interruptAll(); virtual ReplicationCoordinator::StatusAndDuration awaitReplication( - const OperationContext* txn, + OperationContext* txn, const OpTime& opTime, const WriteConcernOptions& writeConcern); virtual ReplicationCoordinator::StatusAndDuration awaitReplicationOfLastOpForClient( - const OperationContext* txn, + OperationContext* txn, const WriteConcernOptions& writeConcern); virtual Status stepDown(OperationContext* txn, @@ -160,7 +160,7 @@ namespace repl { virtual OpTime getMyLastOptime() const override; virtual ReadAfterOpTimeResponse waitUntilOpTime( - const OperationContext* txn, + OperationContext* txn, const ReadAfterOpTimeArgs& settings) override; virtual OID getElectionId() override; @@ -496,7 +496,7 @@ namespace repl { ReplicationCoordinator::StatusAndDuration _awaitReplication_inlock( const Timer* timer, boost::unique_lock<boost::mutex>* lock, - const OperationContext* txn, + OperationContext* txn, const OpTime& opTime, const WriteConcernOptions& writeConcern); diff --git a/src/mongo/db/repl/replication_coordinator_mock.cpp b/src/mongo/db/repl/replication_coordinator_mock.cpp index 9a4e069a747..c5c76b8b7d3 100644 --- a/src/mongo/db/repl/replication_coordinator_mock.cpp +++ b/src/mongo/db/repl/replication_coordinator_mock.cpp @@ -89,7 +89,7 @@ namespace repl { void ReplicationCoordinatorMock::clearSyncSourceBlacklist() {} ReplicationCoordinator::StatusAndDuration ReplicationCoordinatorMock::awaitReplication( - const OperationContext* txn, + OperationContext* txn, const OpTime& opTime, const WriteConcernOptions& writeConcern) { // TODO @@ -98,7 +98,7 @@ namespace repl { ReplicationCoordinator::StatusAndDuration ReplicationCoordinatorMock::awaitReplicationOfLastOpForClient( - const OperationContext* txn, + OperationContext* txn, const WriteConcernOptions& writeConcern) { return StatusAndDuration(Status::OK(), Milliseconds(0)); } @@ -150,7 +150,7 @@ namespace repl { } ReadAfterOpTimeResponse ReplicationCoordinatorMock::waitUntilOpTime( - const OperationContext* txn, + OperationContext* txn, const ReadAfterOpTimeArgs& settings) { return ReadAfterOpTimeResponse(); } diff --git a/src/mongo/db/repl/replication_coordinator_mock.h b/src/mongo/db/repl/replication_coordinator_mock.h index 216a60fcafe..d1a73ea8110 100644 --- a/src/mongo/db/repl/replication_coordinator_mock.h +++ b/src/mongo/db/repl/replication_coordinator_mock.h @@ -65,12 +65,12 @@ namespace repl { virtual void clearSyncSourceBlacklist(); virtual ReplicationCoordinator::StatusAndDuration awaitReplication( - const OperationContext* txn, + OperationContext* txn, const OpTime& opTime, const WriteConcernOptions& writeConcern); virtual ReplicationCoordinator::StatusAndDuration awaitReplicationOfLastOpForClient( - const OperationContext* txn, + OperationContext* txn, const WriteConcernOptions& writeConcern); virtual Status stepDown(OperationContext* txn, @@ -102,7 +102,7 @@ namespace repl { virtual OpTime getMyLastOptime() const; virtual ReadAfterOpTimeResponse waitUntilOpTime( - const OperationContext* txn, + OperationContext* txn, const ReadAfterOpTimeArgs& settings) override; virtual OID getElectionId(); diff --git a/src/mongo/db/service_context_d.cpp b/src/mongo/db/service_context_d.cpp index a9b22a986fb..de3b418e694 100644 --- a/src/mongo/db/service_context_d.cpp +++ b/src/mongo/db/service_context_d.cpp @@ -37,7 +37,6 @@ #include "mongo/base/init.h" #include "mongo/base/initializer.h" #include "mongo/db/client.h" -#include "mongo/db/curop.h" #include "mongo/db/op_observer.h" #include "mongo/db/operation_context_impl.h" #include "mongo/db/storage/storage_engine.h" @@ -46,6 +45,7 @@ #include "mongo/db/storage_options.h" #include "mongo/scripting/engine.h" #include "mongo/stdx/memory.h" +#include "mongo/stdx/mutex.h" #include "mongo/util/log.h" #include "mongo/util/map_util.h" #include "mongo/util/mongoutils/str.h" @@ -235,9 +235,7 @@ namespace mongo { } void ServiceContextMongoD::_killOperation_inlock(OperationContext* opCtx) { - for(CurOp *l = CurOp::get(opCtx); l; l = l->parent()) { - l->kill(); - } + opCtx->markKilled(); for (const auto listener : _killOpListeners) { try { diff --git a/src/mongo/s/d_migrate.cpp b/src/mongo/s/d_migrate.cpp index b3aa9312038..90e9815364e 100644 --- a/src/mongo/s/d_migrate.cpp +++ b/src/mongo/s/d_migrate.cpp @@ -2492,7 +2492,7 @@ namespace { * Returns true if the majority of the nodes and the nodes corresponding to the given * writeConcern (if not empty) have applied till the specified lastOp. */ - bool opReplicatedEnough(const OperationContext* txn, + bool opReplicatedEnough(OperationContext* txn, const repl::OpTime& lastOpApplied, const WriteConcernOptions& writeConcern) { WriteConcernOptions majorityWriteConcern; |