diff options
author | Sean Tao <sean.tao@10gen.com> | 2018-08-09 13:35:50 -0400 |
---|---|---|
committer | Sean Tao <sean.tao@10gen.com> | 2018-08-10 10:36:37 -0400 |
commit | deba67957ddb1c3435fa4c8d36ea9fa3f59c9ad9 (patch) | |
tree | 58cd5bcd87bde74eb8f1f8e78749febdd829ab6f | |
parent | 60a9c20ec95c4cceda9413e18334c55bb85fe66e (diff) | |
download | mongo-deba67957ddb1c3435fa4c8d36ea9fa3f59c9ad9.tar.gz |
SERVER-33366 OperationContext setRecoveryUnit() & releaseRecoveryUnit() should transfer using unique_ptr.
-rw-r--r-- | src/mongo/db/auth/authorization_manager_test.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/concurrency/d_concurrency_test.cpp | 4 | ||||
-rw-r--r-- | src/mongo/db/operation_context.cpp | 8 | ||||
-rw-r--r-- | src/mongo/db/operation_context.h | 4 | ||||
-rw-r--r-- | src/mongo/db/operation_context_noop.h | 5 | ||||
-rw-r--r-- | src/mongo/db/repl/replication_consistency_markers_impl_test.cpp | 3 | ||||
-rw-r--r-- | src/mongo/db/service_context.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp | 5 | ||||
-rw-r--r-- | src/mongo/db/storage/storage_engine_init.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/storage/test_harness_helper.h | 2 | ||||
-rw-r--r-- | src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp | 19 | ||||
-rw-r--r-- | src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit_test.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/transaction_participant.cpp | 15 |
13 files changed, 41 insertions, 33 deletions
diff --git a/src/mongo/db/auth/authorization_manager_test.cpp b/src/mongo/db/auth/authorization_manager_test.cpp index dc3701e2ecf..7e93729aed3 100644 --- a/src/mongo/db/auth/authorization_manager_test.cpp +++ b/src/mongo/db/auth/authorization_manager_test.cpp @@ -352,7 +352,8 @@ public: }; virtual void setUp() override { - opCtx->setRecoveryUnit(recoveryUnit, WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(recoveryUnit), + WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); AuthorizationManagerTest::setUp(); } diff --git a/src/mongo/db/concurrency/d_concurrency_test.cpp b/src/mongo/db/concurrency/d_concurrency_test.cpp index 2738a8db2e6..df07a688104 100644 --- a/src/mongo/db/concurrency/d_concurrency_test.cpp +++ b/src/mongo/db/concurrency/d_concurrency_test.cpp @@ -1658,7 +1658,7 @@ TEST_F(DConcurrencyTestFixture, TestGlobalLockAbandonsSnapshotWhenNotInWriteUnit auto opCtx = clients[0].second.get(); auto recovUnitOwned = stdx::make_unique<RecoveryUnitMock>(); auto recovUnitBorrowed = recovUnitOwned.get(); - opCtx->setRecoveryUnit(recovUnitOwned.release(), + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(recovUnitOwned.release()), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); { @@ -1683,7 +1683,7 @@ TEST_F(DConcurrencyTestFixture, TestGlobalLockDoesNotAbandonSnapshotWhenInWriteU auto opCtx = clients[0].second.get(); auto recovUnitOwned = stdx::make_unique<RecoveryUnitMock>(); auto recovUnitBorrowed = recovUnitOwned.get(); - opCtx->setRecoveryUnit(recovUnitOwned.release(), + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(recovUnitOwned.release()), WriteUnitOfWork::RecoveryUnitState::kActiveUnitOfWork); opCtx->lockState()->beginWriteUnitOfWork(); diff --git a/src/mongo/db/operation_context.cpp b/src/mongo/db/operation_context.cpp index 091ffdd1ee4..94a5ea312d2 100644 --- a/src/mongo/db/operation_context.cpp +++ b/src/mongo/db/operation_context.cpp @@ -379,13 +379,13 @@ void OperationContext::setTxnNumber(TxnNumber txnNumber) { _txnNumber = txnNumber; } -RecoveryUnit* OperationContext::releaseRecoveryUnit() { - return _recoveryUnit.release(); +std::unique_ptr<RecoveryUnit> OperationContext::releaseRecoveryUnit() { + return std::move(_recoveryUnit); } WriteUnitOfWork::RecoveryUnitState OperationContext::setRecoveryUnit( - RecoveryUnit* unit, WriteUnitOfWork::RecoveryUnitState state) { - _recoveryUnit.reset(unit); + std::unique_ptr<RecoveryUnit> unit, WriteUnitOfWork::RecoveryUnitState state) { + _recoveryUnit = std::move(unit); WriteUnitOfWork::RecoveryUnitState oldState = _ruState; _ruState = state; return oldState; diff --git a/src/mongo/db/operation_context.h b/src/mongo/db/operation_context.h index 96c2fecc76e..712a0ab860c 100644 --- a/src/mongo/db/operation_context.h +++ b/src/mongo/db/operation_context.h @@ -97,7 +97,7 @@ public: * We rely on active cursors being killed when collections or databases are dropped, * or when collection metadata changes. */ - RecoveryUnit* releaseRecoveryUnit(); + std::unique_ptr<RecoveryUnit> releaseRecoveryUnit(); /** * Associates the OperatingContext with a different RecoveryUnit for getMore or @@ -105,7 +105,7 @@ public: * returned separately even though the state logically belongs to the RecoveryUnit, * as it is managed by the OperationContext. */ - WriteUnitOfWork::RecoveryUnitState setRecoveryUnit(RecoveryUnit* unit, + WriteUnitOfWork::RecoveryUnitState setRecoveryUnit(std::unique_ptr<RecoveryUnit> unit, WriteUnitOfWork::RecoveryUnitState state); /** diff --git a/src/mongo/db/operation_context_noop.h b/src/mongo/db/operation_context_noop.h index 1f4e51c40f5..dc21cd829bd 100644 --- a/src/mongo/db/operation_context_noop.h +++ b/src/mongo/db/operation_context_noop.h @@ -46,14 +46,15 @@ public: */ OperationContextNoop() : OperationContextNoop(nullptr, 0) {} OperationContextNoop(RecoveryUnit* ru) : OperationContextNoop(nullptr, 0) { - setRecoveryUnit(ru, WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); + setRecoveryUnit(std::unique_ptr<RecoveryUnit>(ru), + WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); } /** * This constructor is for use by ServiceContexts, and should not be called directly. */ OperationContextNoop(Client* client, unsigned int opId) : OperationContext(client, opId) { - setRecoveryUnit(new RecoveryUnitNoop(), + setRecoveryUnit(std::make_unique<RecoveryUnitNoop>(), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); setLockState(std::make_unique<LockerNoop>()); } diff --git a/src/mongo/db/repl/replication_consistency_markers_impl_test.cpp b/src/mongo/db/repl/replication_consistency_markers_impl_test.cpp index d20a8d64423..92f95a35f82 100644 --- a/src/mongo/db/repl/replication_consistency_markers_impl_test.cpp +++ b/src/mongo/db/repl/replication_consistency_markers_impl_test.cpp @@ -272,7 +272,8 @@ TEST_F(ReplicationConsistencyMarkersTest, ReplicationConsistencyMarkers) { // Recovery unit will be owned by "opCtx". RecoveryUnitWithDurabilityTracking* recoveryUnit = new RecoveryUnitWithDurabilityTracking(); - opCtx->setRecoveryUnit(recoveryUnit, WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(recoveryUnit), + WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); // Set min valid without waiting for the changes to be durable. OpTime endOpTime2({Seconds(789), 0}, 1LL); diff --git a/src/mongo/db/service_context.cpp b/src/mongo/db/service_context.cpp index 038cb0a67c4..b14d9332e2b 100644 --- a/src/mongo/db/service_context.cpp +++ b/src/mongo/db/service_context.cpp @@ -235,7 +235,7 @@ ServiceContext::UniqueOperationContext ServiceContext::makeOperationContext(Clie opCtx->setLockState(std::make_unique<LockerNoop>()); } if (!opCtx->recoveryUnit()) { - opCtx->setRecoveryUnit(new RecoveryUnitNoop(), + opCtx->setRecoveryUnit(std::make_unique<RecoveryUnitNoop>(), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); } { diff --git a/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp b/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp index 90c51d942c8..82cb77e8b8b 100644 --- a/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp +++ b/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp @@ -56,8 +56,9 @@ public: Operation() = default; Operation(ServiceContext::UniqueClient client, RecoveryUnit* ru) : _client(std::move(client)), _opCtx(_client->makeOperationContext()) { - delete _opCtx->releaseRecoveryUnit(); - _opCtx->setRecoveryUnit(ru, WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); + _opCtx->releaseRecoveryUnit(); + _opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(ru), + WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); } diff --git a/src/mongo/db/storage/storage_engine_init.cpp b/src/mongo/db/storage/storage_engine_init.cpp index 4805b8105ee..085de0e01c4 100644 --- a/src/mongo/db/storage/storage_engine_init.cpp +++ b/src/mongo/db/storage/storage_engine_init.cpp @@ -330,7 +330,7 @@ public: return; } opCtx->setLockState(stdx::make_unique<LockerImpl>()); - opCtx->setRecoveryUnit(storageEngine->newRecoveryUnit(), + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(storageEngine->newRecoveryUnit()), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); } void onDestroyOperationContext(OperationContext* opCtx) {} diff --git a/src/mongo/db/storage/test_harness_helper.h b/src/mongo/db/storage/test_harness_helper.h index e21947e917c..8513ee6c70b 100644 --- a/src/mongo/db/storage/test_harness_helper.h +++ b/src/mongo/db/storage/test_harness_helper.h @@ -59,7 +59,7 @@ public: virtual ServiceContext::UniqueOperationContext newOperationContext(Client* const client) { auto opCtx = client->makeOperationContext(); - opCtx->setRecoveryUnit(newRecoveryUnit().release(), + opCtx->setRecoveryUnit(newRecoveryUnit(), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); return opCtx; } diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp index e5b1e785527..09b42c028ec 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp @@ -1016,11 +1016,12 @@ int64_t WiredTigerRecordStore::_cappedDeleteAsNeeded_inlock(OperationContext* op const RecordId& justInserted) { // we do this in a side transaction in case it aborts WiredTigerRecoveryUnit* realRecoveryUnit = - checked_cast<WiredTigerRecoveryUnit*>(opCtx->releaseRecoveryUnit()); + checked_cast<WiredTigerRecoveryUnit*>(opCtx->releaseRecoveryUnit().release()); invariant(realRecoveryUnit); WiredTigerSessionCache* sc = realRecoveryUnit->getSessionCache(); - WriteUnitOfWork::RecoveryUnitState const realRUstate = opCtx->setRecoveryUnit( - new WiredTigerRecoveryUnit(sc), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); + WriteUnitOfWork::RecoveryUnitState const realRUstate = + opCtx->setRecoveryUnit(std::make_unique<WiredTigerRecoveryUnit>(sc), + WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); WT_SESSION* session = WiredTigerRecoveryUnit::get(opCtx)->getSession()->getSession(); @@ -1148,18 +1149,18 @@ int64_t WiredTigerRecordStore::_cappedDeleteAsNeeded_inlock(OperationContext* op } } } catch (const WriteConflictException&) { - delete opCtx->releaseRecoveryUnit(); - opCtx->setRecoveryUnit(realRecoveryUnit, realRUstate); + opCtx->releaseRecoveryUnit(); + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(realRecoveryUnit), realRUstate); log() << "got conflict truncating capped, ignoring"; return 0; } catch (...) { - delete opCtx->releaseRecoveryUnit(); - opCtx->setRecoveryUnit(realRecoveryUnit, realRUstate); + opCtx->releaseRecoveryUnit(); + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(realRecoveryUnit), realRUstate); throw; } - delete opCtx->releaseRecoveryUnit(); - opCtx->setRecoveryUnit(realRecoveryUnit, realRUstate); + opCtx->releaseRecoveryUnit(); + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(realRecoveryUnit), realRUstate); return docsRemoved; } diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit_test.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit_test.cpp index 0c3f74516f9..f5c0688dd15 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit_test.cpp +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit_test.cpp @@ -134,7 +134,7 @@ public: auto sc = harnessHelper->serviceContext(); auto client = sc->makeClient(clientName); auto opCtx = client->makeOperationContext(); - opCtx->setRecoveryUnit(harnessHelper->newRecoveryUnit().release(), + opCtx->setRecoveryUnit(harnessHelper->newRecoveryUnit(), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); return std::make_pair(std::move(client), std::move(opCtx)); } diff --git a/src/mongo/db/transaction_participant.cpp b/src/mongo/db/transaction_participant.cpp index 1fa3dcd9b72..aefac7c9c5f 100644 --- a/src/mongo/db/transaction_participant.cpp +++ b/src/mongo/db/transaction_participant.cpp @@ -248,8 +248,9 @@ TransactionParticipant::OplogSlotReserver::OplogSlotReserver(OperationContext* o } // Save the RecoveryUnit from the new transaction and replace it with an empty one. - _recoveryUnit = std::unique_ptr<RecoveryUnit>(opCtx->releaseRecoveryUnit()); - opCtx->setRecoveryUnit(opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit(), + _recoveryUnit = opCtx->releaseRecoveryUnit(); + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>( + opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit()), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); } @@ -281,8 +282,9 @@ TransactionParticipant::TxnResources::TxnResources(OperationContext* opCtx, bool opCtx->lockState()->setMaxLockTimeout(Milliseconds(maxTransactionLockMillis)); } - _recoveryUnit = std::unique_ptr<RecoveryUnit>(opCtx->releaseRecoveryUnit()); - opCtx->setRecoveryUnit(opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit(), + _recoveryUnit = opCtx->releaseRecoveryUnit(); + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>( + opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit()), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); _readConcernArgs = repl::ReadConcernArgs::get(opCtx); @@ -313,7 +315,7 @@ void TransactionParticipant::TxnResources::release(OperationContext* opCtx) { opCtx->swapLockState(std::move(_locker)); opCtx->lockState()->updateThreadIdToCurrentThread(); - auto oldState = opCtx->setRecoveryUnit(_recoveryUnit.release(), + auto oldState = opCtx->setRecoveryUnit(std::move(_recoveryUnit), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); invariant(oldState == WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork, str::stream() << "RecoveryUnit state was " << oldState); @@ -858,7 +860,8 @@ void TransactionParticipant::_cleanUpTxnResourceOnOpCtx(WithLock wl, OperationCo // We must clear the recovery unit and locker so any post-transaction writes can run without // transactional settings such as a read timestamp. - opCtx->setRecoveryUnit(opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit(), + opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>( + opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit()), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork); opCtx->lockState()->unsetMaxLockTimeout(); |