diff options
Diffstat (limited to 'src/mongo/db/storage')
15 files changed, 145 insertions, 152 deletions
diff --git a/src/mongo/db/storage/devnull/devnull_kv_engine.cpp b/src/mongo/db/storage/devnull/devnull_kv_engine.cpp index 3ff3ae5ce96..084761c01ea 100644 --- a/src/mongo/db/storage/devnull/devnull_kv_engine.cpp +++ b/src/mongo/db/storage/devnull/devnull_kv_engine.cpp @@ -111,13 +111,13 @@ public: return StatusWith<RecordId>(RecordId(6, 4)); } - virtual Status updateRecord(OperationContext* txn, - const RecordId& oldLocation, - const char* data, - int len, - bool enforceQuota, - UpdateNotifier* notifier) { - return Status::OK(); + virtual StatusWith<RecordId> updateRecord(OperationContext* txn, + const RecordId& oldLocation, + const char* data, + int len, + bool enforceQuota, + UpdateNotifier* notifier) { + return StatusWith<RecordId>(oldLocation); } virtual bool updateWithDamagesSupported() const { diff --git a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.cpp b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.cpp index 7fa168612b0..6f1c48663dd 100644 --- a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.cpp +++ b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.cpp @@ -430,12 +430,12 @@ StatusWith<RecordId> EphemeralForTestRecordStore::insertRecord(OperationContext* return StatusWith<RecordId>(loc); } -Status EphemeralForTestRecordStore::updateRecord(OperationContext* txn, - const RecordId& loc, - const char* data, - int len, - bool enforceQuota, - UpdateNotifier* notifier) { +StatusWith<RecordId> EphemeralForTestRecordStore::updateRecord(OperationContext* txn, + const RecordId& loc, + const char* data, + int len, + bool enforceQuota, + UpdateNotifier* notifier) { EphemeralForTestRecord* oldRecord = recordFor(loc); int oldLen = oldRecord->size; @@ -447,7 +447,7 @@ Status EphemeralForTestRecordStore::updateRecord(OperationContext* txn, // doc-locking), and therefore must notify that it is updating a document. Status callbackStatus = notifier->recordStoreGoingToUpdateInPlace(txn, loc); if (!callbackStatus.isOK()) { - return callbackStatus; + return StatusWith<RecordId>(callbackStatus); } } @@ -460,7 +460,7 @@ Status EphemeralForTestRecordStore::updateRecord(OperationContext* txn, cappedDeleteAsNeeded(txn); - return Status::OK(); + return StatusWith<RecordId>(loc); } bool EphemeralForTestRecordStore::updateWithDamagesSupported() const { diff --git a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.h b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.h index 8f83cfe9dfd..324a30653eb 100644 --- a/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.h +++ b/src/mongo/db/storage/ephemeral_for_test/ephemeral_for_test_record_store.h @@ -69,12 +69,12 @@ public: const DocWriter* doc, bool enforceQuota); - virtual Status updateRecord(OperationContext* txn, - const RecordId& oldLocation, - const char* data, - int len, - bool enforceQuota, - UpdateNotifier* notifier); + virtual StatusWith<RecordId> updateRecord(OperationContext* txn, + const RecordId& oldLocation, + const char* data, + int len, + bool enforceQuota, + UpdateNotifier* notifier); virtual bool updateWithDamagesSupported() const; diff --git a/src/mongo/db/storage/kv/kv_catalog.cpp b/src/mongo/db/storage/kv/kv_catalog.cpp index bff423434b4..6a6d1a8e437 100644 --- a/src/mongo/db/storage/kv/kv_catalog.cpp +++ b/src/mongo/db/storage/kv/kv_catalog.cpp @@ -290,8 +290,10 @@ void KVCatalog::putMetaData(OperationContext* opCtx, } LOG(3) << "recording new metadata: " << obj; - Status status = _rs->updateRecord(opCtx, loc, obj.objdata(), obj.objsize(), false, NULL); - fassert(28521, status.isOK()); + StatusWith<RecordId> status = + _rs->updateRecord(opCtx, loc, obj.objdata(), obj.objsize(), false, NULL); + fassert(28521, status.getStatus()); + invariant(status.getValue() == loc); } Status KVCatalog::renameCollection(OperationContext* opCtx, @@ -320,8 +322,10 @@ Status KVCatalog::renameCollection(OperationContext* opCtx, b.appendElementsUnique(old); BSONObj obj = b.obj(); - Status status = _rs->updateRecord(opCtx, loc, obj.objdata(), obj.objsize(), false, NULL); - fassert(28522, status.isOK()); + StatusWith<RecordId> status = + _rs->updateRecord(opCtx, loc, obj.objdata(), obj.objsize(), false, NULL); + fassert(28522, status.getStatus()); + invariant(status.getValue() == loc); } stdx::lock_guard<stdx::mutex> lk(_identsLock); diff --git a/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp b/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp index 20d5e99f89c..278474cabb0 100644 --- a/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp +++ b/src/mongo/db/storage/mmap_v1/catalog/namespace_details_collection_entry.cpp @@ -358,30 +358,12 @@ void NamespaceDetailsCollectionCatalogEntry::_updateSystemNamespaces(OperationCo if (!_namespacesRecordStore) return; - RecordId currentRecordId = _namespacesRecordId; - - RecordData entry = _namespacesRecordStore->dataFor(txn, currentRecordId); + RecordData entry = _namespacesRecordStore->dataFor(txn, _namespacesRecordId); const BSONObj newEntry = applyUpdateOperators(entry.releaseToBson(), update); - - Status result = _namespacesRecordStore->updateRecord( - txn, currentRecordId, newEntry.objdata(), newEntry.objsize(), false, NULL); - - if (ErrorCodes::NeedsDocumentMove == result) { - StatusWith<RecordId> newLocation = _namespacesRecordStore->insertRecord( - txn, newEntry.objdata(), newEntry.objsize(), false); - fassert(40051, newLocation.getStatus().isOK()); - currentRecordId = newLocation.getValue(); - setNamespacesRecordId(txn, currentRecordId); - - // Intentionally not deleting the old MMAPv1 record on move. The reasoning for this is: - // - It might be possible that there are other parts in the code base that reference this - // RecordId and removing could introduce an MMAPv1 bug. - // - It is not harmful leaving the old RecordId in place. On document move, the space - // allocated for the new document is double the old. This puts a practical limit on the - // potential number of old 'leaked' documents. - } else { - fassert(17486, result.isOK()); - } + StatusWith<RecordId> result = _namespacesRecordStore->updateRecord( + txn, _namespacesRecordId, newEntry.objdata(), newEntry.objsize(), false, NULL); + fassert(17486, result.getStatus()); + setNamespacesRecordId(txn, result.getValue()); } void NamespaceDetailsCollectionCatalogEntry::updateFlags(OperationContext* txn, int newValue) { diff --git a/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h b/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h index 4e984a4469d..a496f1ff31e 100644 --- a/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h +++ b/src/mongo/db/storage/mmap_v1/heap_record_store_btree.h @@ -74,12 +74,12 @@ public: // ------------------------------ - virtual Status updateRecord(OperationContext* txn, - const RecordId& oldLocation, - const char* data, - int len, - bool enforceQuota, - UpdateNotifier* notifier) { + virtual StatusWith<RecordId> updateRecord(OperationContext* txn, + const RecordId& oldLocation, + const char* data, + int len, + bool enforceQuota, + UpdateNotifier* notifier) { invariant(false); } diff --git a/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp b/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp index f5089ef787f..0105d10c4df 100644 --- a/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp +++ b/src/mongo/db/storage/mmap_v1/record_store_v1_base.cpp @@ -367,30 +367,49 @@ StatusWith<RecordId> RecordStoreV1Base::_insertRecord(OperationContext* txn, return StatusWith<RecordId>(loc.getValue().toRecordId()); } -Status RecordStoreV1Base::updateRecord(OperationContext* txn, - const RecordId& oldLocation, - const char* data, - int dataSize, - bool enforceQuota, - UpdateNotifier* notifier) { +StatusWith<RecordId> RecordStoreV1Base::updateRecord(OperationContext* txn, + const RecordId& oldLocation, + const char* data, + int dataSize, + bool enforceQuota, + UpdateNotifier* notifier) { MmapV1RecordHeader* oldRecord = recordFor(DiskLoc::fromRecordId(oldLocation)); if (oldRecord->netLength() >= dataSize) { // Make sure to notify other queries before we do an in-place update. if (notifier) { Status callbackStatus = notifier->recordStoreGoingToUpdateInPlace(txn, oldLocation); if (!callbackStatus.isOK()) - return callbackStatus; + return StatusWith<RecordId>(callbackStatus); } // we fit memcpy(txn->recoveryUnit()->writingPtr(oldRecord->data(), dataSize), data, dataSize); - return Status::OK(); + return StatusWith<RecordId>(oldLocation); } // We enforce the restriction of unchanging capped doc sizes above the storage layer. invariant(!isCapped()); - return {ErrorCodes::NeedsDocumentMove, "Update requires document move"}; + // we have to move + if (dataSize + MmapV1RecordHeader::HeaderSize > MaxAllowedAllocation) { + return StatusWith<RecordId>(ErrorCodes::InvalidLength, "record has to be <= 16.5MB"); + } + + StatusWith<RecordId> newLocation = _insertRecord(txn, data, dataSize, enforceQuota); + if (!newLocation.isOK()) + return newLocation; + + // insert worked, so we delete old record + if (notifier) { + Status moveStatus = notifier->recordStoreGoingToMove( + txn, oldLocation, oldRecord->data(), oldRecord->netLength()); + if (!moveStatus.isOK()) + return StatusWith<RecordId>(moveStatus); + } + + deleteRecord(txn, oldLocation); + + return newLocation; } bool RecordStoreV1Base::updateWithDamagesSupported() const { diff --git a/src/mongo/db/storage/mmap_v1/record_store_v1_base.h b/src/mongo/db/storage/mmap_v1/record_store_v1_base.h index 4520256f901..d34c5a9b3f0 100644 --- a/src/mongo/db/storage/mmap_v1/record_store_v1_base.h +++ b/src/mongo/db/storage/mmap_v1/record_store_v1_base.h @@ -199,12 +199,12 @@ public: const DocWriter* doc, bool enforceQuota); - virtual Status updateRecord(OperationContext* txn, - const RecordId& oldLocation, - const char* data, - int len, - bool enforceQuota, - UpdateNotifier* notifier); + virtual StatusWith<RecordId> updateRecord(OperationContext* txn, + const RecordId& oldLocation, + const char* data, + int len, + bool enforceQuota, + UpdateNotifier* notifier); virtual bool updateWithDamagesSupported() const; diff --git a/src/mongo/db/storage/record_store.h b/src/mongo/db/storage/record_store.h index 64f93e50c62..2beb8368bf4 100644 --- a/src/mongo/db/storage/record_store.h +++ b/src/mongo/db/storage/record_store.h @@ -76,6 +76,10 @@ public: class UpdateNotifier { public: virtual ~UpdateNotifier() {} + virtual Status recordStoreGoingToMove(OperationContext* txn, + const RecordId& oldLocation, + const char* oldBuffer, + size_t oldSize) = 0; virtual Status recordStoreGoingToUpdateInPlace(OperationContext* txn, const RecordId& loc) = 0; }; @@ -382,23 +386,22 @@ public: } /** - * @param notifier - Only used by record stores which do not support doc-locking. Called only - * in the case of an in-place update. Called just before the in-place write - * occurs. - * @return Status - If a document move is required (MMAPv1 only) then a status of - * ErrorCodes::NeedsDocumentMove will be returned. On receipt of this status - * no update will be performed. It is the caller's responsibility to: - * 1. Remove the existing document and associated index keys. - * 2. Insert a new document and index keys. + * @param notifier - Only used by record stores which do not support doc-locking. + * In the case of a document move, this is called after the document + * has been written to the new location, but before it is deleted from + * the old location. + * In the case of an in-place update, this is called just before the + * in-place write occurs. + * @return Status or RecordId, RecordId might be different * * For capped record stores, the record size will never change. */ - virtual Status updateRecord(OperationContext* txn, - const RecordId& oldLocation, - const char* data, - int len, - bool enforceQuota, - UpdateNotifier* notifier) = 0; + virtual StatusWith<RecordId> updateRecord(OperationContext* txn, + const RecordId& oldLocation, + const char* data, + int len, + bool enforceQuota, + UpdateNotifier* notifier) = 0; /** * @return Returns 'false' if this record store does not implement diff --git a/src/mongo/db/storage/record_store_test_harness.cpp b/src/mongo/db/storage/record_store_test_harness.cpp index 8fc234ab9cd..95fd4c993b0 100644 --- a/src/mongo/db/storage/record_store_test_harness.cpp +++ b/src/mongo/db/storage/record_store_test_harness.cpp @@ -257,23 +257,10 @@ TEST(RecordStoreTestHarness, Update1) { unique_ptr<OperationContext> opCtx(harnessHelper->newOperationContext()); { WriteUnitOfWork uow(opCtx.get()); - Status status = + StatusWith<RecordId> res = rs->updateRecord(opCtx.get(), loc, s2.c_str(), s2.size() + 1, false, NULL); - - if (ErrorCodes::NeedsDocumentMove == status) { - // NeedsDocumentMove should only be possible under MMAPv1. We don't have the means - // to check storageEngine here but asserting 'supportsDocLocking()' is false - // provides an equivalent check as only MMAPv1 will/should return false. - ASSERT_FALSE(harnessHelper->supportsDocLocking()); - StatusWith<RecordId> newLocation = - rs->insertRecord(opCtx.get(), s2.c_str(), s2.size() + 1, false); - ASSERT_OK(newLocation.getStatus()); - rs->deleteRecord(opCtx.get(), loc); - loc = newLocation.getValue(); - } else { - ASSERT_OK(status); - } - + ASSERT_OK(res.getStatus()); + loc = res.getValue(); uow.commit(); } } diff --git a/src/mongo/db/storage/record_store_test_updaterecord.cpp b/src/mongo/db/storage/record_store_test_updaterecord.cpp index cd27acf9c69..0d7c9433503 100644 --- a/src/mongo/db/storage/record_store_test_updaterecord.cpp +++ b/src/mongo/db/storage/record_store_test_updaterecord.cpp @@ -77,19 +77,10 @@ TEST(RecordStoreTestHarness, UpdateRecord) { unique_ptr<OperationContext> opCtx(harnessHelper->newOperationContext()); { WriteUnitOfWork uow(opCtx.get()); - Status res = + StatusWith<RecordId> res = rs->updateRecord(opCtx.get(), loc, data.c_str(), data.size() + 1, false, NULL); - - if (ErrorCodes::NeedsDocumentMove == res) { - StatusWith<RecordId> newLocation = - rs->insertRecord(opCtx.get(), data.c_str(), data.size() + 1, false); - ASSERT_OK(newLocation.getStatus()); - rs->deleteRecord(opCtx.get(), loc); - loc = newLocation.getValue(); - } else { - ASSERT_OK(res); - } - + ASSERT_OK(res.getStatus()); + loc = res.getValue(); uow.commit(); } } @@ -145,19 +136,10 @@ TEST(RecordStoreTestHarness, UpdateMultipleRecords) { string data = ss.str(); WriteUnitOfWork uow(opCtx.get()); - Status res = + StatusWith<RecordId> res = rs->updateRecord(opCtx.get(), locs[i], data.c_str(), data.size() + 1, false, NULL); - - if (ErrorCodes::NeedsDocumentMove == res) { - StatusWith<RecordId> newLocation = - rs->insertRecord(opCtx.get(), data.c_str(), data.size() + 1, false); - ASSERT_OK(newLocation.getStatus()); - rs->deleteRecord(opCtx.get(), locs[i]); - locs[i] = newLocation.getValue(); - } else { - ASSERT_OK(res); - } - + ASSERT_OK(res.getStatus()); + locs[i] = res.getValue(); uow.commit(); } } @@ -212,21 +194,21 @@ TEST(RecordStoreTestHarness, UpdateRecordWithMoveNotifier) { UpdateNotifierSpy umn(opCtx.get(), loc, oldData.c_str(), oldData.size()); WriteUnitOfWork uow(opCtx.get()); - Status res = rs->updateRecord( + StatusWith<RecordId> res = rs->updateRecord( opCtx.get(), loc, newData.c_str(), newData.size() + 1, false, &umn); - - if (ErrorCodes::NeedsDocumentMove == res) { - StatusWith<RecordId> newLocation = - rs->insertRecord(opCtx.get(), newData.c_str(), newData.size() + 1, false); - ASSERT_OK(newLocation.getStatus()); - rs->deleteRecord(opCtx.get(), loc); - loc = newLocation.getValue(); - ASSERT_EQUALS(0, umn.numInPlaceCallbacks()); - } else { - ASSERT_OK(res); + ASSERT_OK(res.getStatus()); + // UpdateNotifier::recordStoreGoingToMove() called only if + // the RecordId for the record changes + if (loc == res.getValue()) { + ASSERT_EQUALS(0, umn.numMoveCallbacks()); + // Only MMAP v1 is required to use the UpdateNotifier for in-place updates, + // so the number of callbacks is expected to be 0 for non-MMAP storage engines. ASSERT_GTE(1, umn.numInPlaceCallbacks()); + } else { + ASSERT_EQUALS(1, umn.numMoveCallbacks()); + ASSERT_EQUALS(0, umn.numInPlaceCallbacks()); } - + loc = res.getValue(); uow.commit(); } } diff --git a/src/mongo/db/storage/record_store_test_updaterecord.h b/src/mongo/db/storage/record_store_test_updaterecord.h index be52887cf2b..f82feb6b592 100644 --- a/src/mongo/db/storage/record_store_test_updaterecord.h +++ b/src/mongo/db/storage/record_store_test_updaterecord.h @@ -41,10 +41,21 @@ namespace { class UpdateNotifierSpy : public UpdateNotifier { public: UpdateNotifierSpy(OperationContext* txn, const RecordId& loc, const char* buf, size_t size) - : _txn(txn), _loc(loc), _data(buf, size), nInPlaceCalls(0) {} + : _txn(txn), _loc(loc), _data(buf, size), nMoveCalls(0), nInPlaceCalls(0) {} ~UpdateNotifierSpy() {} + Status recordStoreGoingToMove(OperationContext* txn, + const RecordId& oldLocation, + const char* oldBuffer, + size_t oldSize) { + nMoveCalls++; + ASSERT_EQUALS(_txn, txn); + ASSERT_EQUALS(_loc, oldLocation); + ASSERT_EQUALS(_data, oldBuffer); + return Status::OK(); + } + Status recordStoreGoingToUpdateInPlace(OperationContext* txn, const RecordId& loc) { nInPlaceCalls++; ASSERT_EQUALS(_txn, txn); @@ -52,6 +63,10 @@ public: return Status::OK(); } + int numMoveCallbacks() const { + return nMoveCalls; + } + int numInPlaceCallbacks() const { return nInPlaceCalls; } @@ -62,6 +77,7 @@ private: std::string _data; // To verify the number of callbacks to the notifier. + int nMoveCalls; int nInPlaceCalls; }; diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp index 636f2d86ffd..f2ebc977f34 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp @@ -1367,12 +1367,12 @@ StatusWith<RecordId> WiredTigerRecordStore::insertRecord(OperationContext* txn, return insertRecord(txn, buf.get(), len, enforceQuota); } -Status WiredTigerRecordStore::updateRecord(OperationContext* txn, - const RecordId& id, - const char* data, - int len, - bool enforceQuota, - UpdateNotifier* notifier) { +StatusWith<RecordId> WiredTigerRecordStore::updateRecord(OperationContext* txn, + const RecordId& id, + const char* data, + int len, + bool enforceQuota, + UpdateNotifier* notifier) { WiredTigerCursor curwrap(_uri, _tableId, true, txn); curwrap.assertInActiveTxn(); WT_CURSOR* c = curwrap.get(); @@ -1402,7 +1402,7 @@ Status WiredTigerRecordStore::updateRecord(OperationContext* txn, cappedDeleteAsNeeded(txn, id); } - return Status::OK(); + return StatusWith<RecordId>(id); } bool WiredTigerRecordStore::updateWithDamagesSupported() const { diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h index 9e7cc01f276..3335e774c4c 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.h @@ -131,12 +131,12 @@ public: const DocWriter* doc, bool enforceQuota); - virtual Status updateRecord(OperationContext* txn, - const RecordId& oldLocation, - const char* data, - int len, - bool enforceQuota, - UpdateNotifier* notifier); + virtual StatusWith<RecordId> updateRecord(OperationContext* txn, + const RecordId& oldLocation, + const char* data, + int len, + bool enforceQuota, + UpdateNotifier* notifier); virtual bool updateWithDamagesSupported() const; diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store_test.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store_test.cpp index e09ccdf3e65..7dcc4033f70 100644 --- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store_test.cpp +++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store_test.cpp @@ -239,8 +239,8 @@ TEST(WiredTigerRecordStoreTest, Isolation1) { rs->dataFor(t1.get(), id1); rs->dataFor(t2.get(), id1); - ASSERT_OK(rs->updateRecord(t1.get(), id1, "b", 2, false, NULL)); - ASSERT_OK(rs->updateRecord(t1.get(), id2, "B", 2, false, NULL)); + ASSERT_OK(rs->updateRecord(t1.get(), id1, "b", 2, false, NULL).getStatus()); + ASSERT_OK(rs->updateRecord(t1.get(), id2, "B", 2, false, NULL).getStatus()); try { // this should fail @@ -289,7 +289,7 @@ TEST(WiredTigerRecordStoreTest, Isolation2) { { WriteUnitOfWork w(t1.get()); - ASSERT_OK(rs->updateRecord(t1.get(), id1, "b", 2, false, NULL)); + ASSERT_OK(rs->updateRecord(t1.get(), id1, "b", 2, false, NULL).getStatus()); w.commit(); } |