From 2b594bdab554e569a94cd4e9af6809a51dd881ad Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 8 Apr 2019 22:43:29 +0200 Subject: [PATCH] Reduce # of memcpy from storage engine layer (#8685) --- .../6.0.fb/monitoring/iostats_context_imp.h | 1 + arangod/ClusterEngine/ClusterCollection.cpp | 16 +- arangod/ClusterEngine/ClusterCollection.h | 16 +- arangod/MMFiles/MMFilesCollection.cpp | 163 +++---- arangod/MMFiles/MMFilesCollection.h | 29 +- arangod/MMFiles/MMFilesDocumentOperation.cpp | 3 +- arangod/MMFiles/MMFilesDocumentOperation.h | 4 - arangod/MMFiles/MMFilesIndexLookupContext.cpp | 2 +- arangod/MMFiles/MMFilesTransactionState.cpp | 12 +- arangod/MMFiles/MMFilesTransactionState.h | 5 + arangod/Replication/utilities.cpp | 2 +- arangod/RocksDBEngine/RocksDBCollection.cpp | 460 +++++++++--------- arangod/RocksDBEngine/RocksDBCollection.h | 51 +- arangod/RocksDBEngine/RocksDBEdgeIndex.cpp | 28 +- .../RocksDBEngine/RocksDBIncrementalSync.cpp | 45 +- arangod/RocksDBEngine/RocksDBPrimaryIndex.h | 3 +- arangod/RocksDBEngine/RocksDBVPackIndex.cpp | 12 +- arangod/StorageEngine/PhysicalCollection.h | 37 +- arangod/Transaction/Context.cpp | 22 +- arangod/Transaction/Context.h | 8 +- arangod/Transaction/Methods.cpp | 143 ++---- arangod/Transaction/Methods.h | 9 +- arangod/VocBase/LogicalCollection.cpp | 40 +- arangod/VocBase/LogicalCollection.h | 29 +- arangod/VocBase/ManagedDocumentResult.cpp | 41 +- arangod/VocBase/ManagedDocumentResult.h | 53 +- js/client/modules/@arangodb/test-utils.js | 5 +- tests/IResearch/IResearchQueryJoin-test.cpp | 14 +- .../IResearchQueryNumericTerm-test.cpp | 4 +- .../IResearchQueryOptimization-test.cpp | 4 +- .../IResearch/IResearchQueryOptions-test.cpp | 8 +- tests/IResearch/IResearchQueryOr-test.cpp | 4 +- tests/IResearch/IResearchQueryScorer-test.cpp | 4 +- .../IResearchQuerySelectAll-test.cpp | 6 +- .../IResearchQueryStartsWith-test.cpp | 4 +- .../IResearchQueryStringTerm-test.cpp | 4 +- tests/IResearch/IResearchView-test.cpp | 4 +- .../IResearch/IResearchViewDBServer-test.cpp | 4 +- tests/IResearch/IResearchViewNode-test.cpp | 2 +- tests/Mocks/StorageEngineMock.cpp | 53 +- tests/Mocks/StorageEngineMock.h | 22 +- 41 files changed, 651 insertions(+), 725 deletions(-) diff --git a/3rdParty/rocksdb/6.0.fb/monitoring/iostats_context_imp.h b/3rdParty/rocksdb/6.0.fb/monitoring/iostats_context_imp.h index 60370daaee..23c2088cab 100644 --- a/3rdParty/rocksdb/6.0.fb/monitoring/iostats_context_imp.h +++ b/3rdParty/rocksdb/6.0.fb/monitoring/iostats_context_imp.h @@ -55,5 +55,6 @@ extern __thread IOStatsContext iostats_context; #define IOSTATS(metric) 0 #define IOSTATS_TIMER_GUARD(metric) +#define IOSTATS_CPU_TIMER_GUARD(metric, env) #endif // ROCKSDB_SUPPORT_THREAD_LOCAL diff --git a/arangod/ClusterEngine/ClusterCollection.cpp b/arangod/ClusterEngine/ClusterCollection.cpp index 535e73af4c..11070f3cd9 100644 --- a/arangod/ClusterEngine/ClusterCollection.cpp +++ b/arangod/ClusterEngine/ClusterCollection.cpp @@ -452,33 +452,29 @@ bool ClusterCollection::readDocumentWithCallback(transaction::Methods* trx, Result ClusterCollection::insert(arangodb::transaction::Methods*, arangodb::velocypack::Slice const, arangodb::ManagedDocumentResult&, - OperationOptions&, TRI_voc_tick_t&, bool, - TRI_voc_tick_t&, KeyLockInfo* /*keyLock*/, - std::function const&) { + OperationOptions&, bool /*lock*/, + KeyLockInfo* /*keyLock*/, + std::function const&) { THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED); } Result ClusterCollection::update(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, ManagedDocumentResult& mdr, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) { + bool /*lock*/, ManagedDocumentResult& previous) { THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED); } Result ClusterCollection::replace(transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, ManagedDocumentResult& mdr, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) { + bool /*lock*/, ManagedDocumentResult& previous) { THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED); } Result ClusterCollection::remove(transaction::Methods& trx, velocypack::Slice slice, ManagedDocumentResult& previous, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, TRI_voc_rid_t& revisionId, - KeyLockInfo* /*keyLock*/, std::function const& /*callbackDuringLock*/ + bool /*lock*/, KeyLockInfo* /*keyLock*/, std::function const& /*callbackDuringLock*/ ) { THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED); } diff --git a/arangod/ClusterEngine/ClusterCollection.h b/arangod/ClusterEngine/ClusterCollection.h index 3bfb4d7570..63e469124e 100644 --- a/arangod/ClusterEngine/ClusterCollection.h +++ b/arangod/ClusterEngine/ClusterCollection.h @@ -144,25 +144,21 @@ class ClusterCollection final : public PhysicalCollection { Result insert(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice newSlice, arangodb::ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_tick_t& revisionId, KeyLockInfo* /*keyLockInfo*/, - std::function const& callbackDuringLock) override; + bool lock, KeyLockInfo* /*keyLockInfo*/, + std::function const& callbackDuringLock) override; Result update(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& prevRev, - ManagedDocumentResult& previous) override; + bool lock, ManagedDocumentResult& previous) override; Result replace(transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) override; + bool lock, ManagedDocumentResult& previous) override; Result remove(transaction::Methods& trx, velocypack::Slice slice, ManagedDocumentResult& previous, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& prevRev, - TRI_voc_rid_t& revisionId, KeyLockInfo* keyLockInfo, - std::function const& callbackDuringLock) override; + bool lock, KeyLockInfo* keyLockInfo, + std::function const& callbackDuringLock) override; protected: /// @brief Inject figures that are specific to StorageEngine diff --git a/arangod/MMFiles/MMFilesCollection.cpp b/arangod/MMFiles/MMFilesCollection.cpp index 97a59cd6b9..ad494c998a 100644 --- a/arangod/MMFiles/MMFilesCollection.cpp +++ b/arangod/MMFiles/MMFilesCollection.cpp @@ -2014,9 +2014,9 @@ Result MMFilesCollection::read(transaction::Methods* trx, VPackSlice const& key, } TRI_DEFER(if (lock) { unlockRead(useDeadlockDetector, trx->state()); }); - Result res = lookupDocument(trx, key, result); - if (res.fail()) { - return res; + LocalDocumentId const documentId = lookupDocument(trx, key, result); + if (documentId.empty()) { + return Result(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND); } // we found a document @@ -2037,7 +2037,7 @@ bool MMFilesCollection::readDocument(transaction::Methods* trx, ManagedDocumentResult& result) const { uint8_t const* vpack = lookupDocumentVPack(documentId); if (vpack != nullptr) { - result.setUnmanaged(vpack, documentId); + result.setUnmanaged(vpack); return true; } return false; @@ -2076,7 +2076,7 @@ bool MMFilesCollection::readDocumentConditional(transaction::Methods* trx, TRI_ASSERT(documentId.isSet()); uint8_t const* vpack = lookupDocumentVPackConditional(documentId, maxTick, true); if (vpack != nullptr) { - result.setUnmanaged(vpack, documentId); + result.setUnmanaged(vpack); return true; } return false; @@ -2831,17 +2831,17 @@ LocalDocumentId MMFilesCollection::reuseOrCreateLocalDocumentId(OperationOptions } Result MMFilesCollection::insert(arangodb::transaction::Methods* trx, VPackSlice const slice, - arangodb::ManagedDocumentResult& result, + arangodb::ManagedDocumentResult& resultMdr, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_tick_t& revisionId, KeyLockInfo* keyLockInfo, - std::function const& callbackDuringLock) { + bool lock, KeyLockInfo* keyLockInfo, + std::function const& callbackDuringLock) { LocalDocumentId const documentId = reuseOrCreateLocalDocumentId(options); auto isEdgeCollection = (TRI_COL_TYPE_EDGE == _logicalCollection.type()); transaction::BuilderLeaser builder(trx); VPackSlice newSlice; Result res(TRI_ERROR_NO_ERROR); + TRI_voc_rid_t revisionId = 0; if (options.recoveryData == nullptr) { res = newObjectForInsert(trx, slice, isEdgeCollection, *builder.get(), options.isRestore, revisionId); @@ -2871,11 +2871,12 @@ Result MMFilesCollection::insert(arangodb::transaction::Methods* trx, VPackSlice if (revSlice.isString()) { VPackValueLength l; - char const* p = revSlice.getString(l); + char const* p = revSlice.getStringUnchecked(l); TRI_ASSERT(p != nullptr); revisionId = TRI_StringToRid(p, l, false); } } + TRI_ASSERT(revisionId != 0); // create marker MMFilesCrudMarker insertMarker( @@ -2957,7 +2958,7 @@ Result MMFilesCollection::insert(arangodb::transaction::Methods* trx, VPackSlice } if (res.ok() && callbackDuringLock != nullptr) { - res = callbackDuringLock(); + callbackDuringLock(); } } catch (...) { // the collectionLocker may have thrown in its constructor... @@ -2976,9 +2977,18 @@ Result MMFilesCollection::insert(arangodb::transaction::Methods* trx, VPackSlice } if (res.ok()) { - result.setManaged(doc.begin(), documentId); - // store the tick that was used for writing the document - resultMarkerTick = operation.tick(); + if (options.returnNew) { + resultMdr.setManaged(doc.begin()); + TRI_ASSERT(resultMdr.revisionId() == revisionId); + } else if (!options.silent) { // need to pass up revision and key + transaction::BuilderLeaser keyBuilder(trx); + keyBuilder->openObject(/*unindexed*/true); + keyBuilder->add(StaticStrings::KeyString, transaction::helpers::extractKeyFromDocument(doc)); + keyBuilder->close(); + resultMdr.setManaged()->assign(reinterpret_cast(keyBuilder->start()), + keyBuilder->size()); + resultMdr.setRevisionId(revisionId); + } } return res; @@ -3375,13 +3385,14 @@ Result MMFilesCollection::insertDocument(arangodb::transaction::Methods& trx, } Result MMFilesCollection::update(arangodb::transaction::Methods* trx, - VPackSlice const newSlice, ManagedDocumentResult& result, - OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool lock, TRI_voc_rid_t& prevRev, - ManagedDocumentResult& previous) { + VPackSlice const newSlice, ManagedDocumentResult& resultMdr, + OperationOptions& options, + bool lock, ManagedDocumentResult& previousMdr) { VPackSlice key = newSlice.get(StaticStrings::KeyString); if (key.isNone()) { return Result(TRI_ERROR_ARANGO_DOCUMENT_HANDLE_BAD); + } else if (!key.isString()) { + return Result(TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD); } LocalDocumentId const documentId = reuseOrCreateLocalDocumentId(options); @@ -3396,16 +3407,13 @@ Result MMFilesCollection::update(arangodb::transaction::Methods* trx, trx->state(), lock); // get the previous revision - Result res = lookupDocument(trx, key, previous); - - if (res.fail()) { - return res; + LocalDocumentId const oldDocumentId = lookupDocument(trx, key, previousMdr); + if (oldDocumentId.empty()) { + return Result(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND); } - LocalDocumentId const oldDocumentId = previous.localDocumentId(); - VPackSlice const oldDoc(previous.vpack()); - TRI_voc_rid_t const oldRevisionId = transaction::helpers::extractRevFromDocument(oldDoc); - prevRev = oldRevisionId; + VPackSlice const oldDoc(previousMdr.vpack()); + TRI_ASSERT(previousMdr.revisionId() != 0); TRI_IF_FAILURE("UpdateDocumentNoMarker") { // test what happens when no marker can be created @@ -3423,7 +3431,7 @@ Result MMFilesCollection::update(arangodb::transaction::Methods* trx, if (newSlice.isObject()) { expectedRev = TRI_ExtractRevisionId(newSlice); } - int res = checkRevision(trx, expectedRev, prevRev); + int res = checkRevision(trx, expectedRev, previousMdr.revisionId()); if (res != TRI_ERROR_NO_ERROR) { return Result(res); } @@ -3431,7 +3439,7 @@ Result MMFilesCollection::update(arangodb::transaction::Methods* trx, if (newSlice.length() <= 1) { // no need to do anything - result = previous; + resultMdr = previousMdr; if (_logicalCollection.waitForSync()) { options.waitForSync = true; @@ -3440,6 +3448,7 @@ Result MMFilesCollection::update(arangodb::transaction::Methods* trx, return Result(); } + Result res; // merge old and new values TRI_voc_rid_t revisionId; transaction::BuilderLeaser builder(trx); @@ -3503,11 +3512,10 @@ Result MMFilesCollection::update(arangodb::transaction::Methods* trx, if (res.fail()) { operation.revert(trx); } else { - result.setManaged(newDoc.begin(), documentId); - - if (options.waitForSync) { - // store the tick that was used for writing the new document - resultMarkerTick = operation.tick(); + if (options.returnNew) { + resultMdr.setManaged(newDoc.begin()); + } else { // need to pass revId manually + resultMdr.setRevisionId(revisionId); } } @@ -3515,9 +3523,8 @@ Result MMFilesCollection::update(arangodb::transaction::Methods* trx, } Result MMFilesCollection::replace(transaction::Methods* trx, VPackSlice const newSlice, - ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) { + ManagedDocumentResult& resultMdr, OperationOptions& options, + bool lock, ManagedDocumentResult& previousMdr) { LocalDocumentId const documentId = reuseOrCreateLocalDocumentId(options); auto isEdgeCollection = (TRI_COL_TYPE_EDGE == _logicalCollection.type()); @@ -3527,6 +3534,8 @@ Result MMFilesCollection::replace(transaction::Methods* trx, VPackSlice const ne VPackSlice key = newSlice.get(StaticStrings::KeyString); if (key.isNone()) { return TRI_ERROR_ARANGO_DOCUMENT_HANDLE_BAD; + } else if (!key.isString()) { + return Result(TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD); } bool const useDeadlockDetector = @@ -3536,10 +3545,9 @@ Result MMFilesCollection::replace(transaction::Methods* trx, VPackSlice const ne trx->state(), lock); // get the previous revision - Result res = lookupDocument(trx, key, previous); - - if (res.fail()) { - return res; + LocalDocumentId const oldDocumentId = lookupDocument(trx, key, previousMdr); + if (oldDocumentId.empty()) { + return Result(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND); } TRI_IF_FAILURE("ReplaceDocumentNoMarker") { @@ -3552,12 +3560,10 @@ Result MMFilesCollection::replace(transaction::Methods* trx, VPackSlice const ne THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG); } - uint8_t const* vpack = previous.vpack(); - LocalDocumentId const oldDocumentId = previous.localDocumentId(); + uint8_t const* vpack = previousMdr.vpack(); + TRI_ASSERT(previousMdr.revisionId() != 0); - VPackSlice oldDoc(vpack); - TRI_voc_rid_t oldRevisionId = transaction::helpers::extractRevFromDocument(oldDoc); - prevRev = oldRevisionId; + VPackSlice const oldDoc(vpack); // Check old revision: if (!options.ignoreRevs) { @@ -3565,12 +3571,13 @@ Result MMFilesCollection::replace(transaction::Methods* trx, VPackSlice const ne if (newSlice.isObject()) { expectedRev = TRI_ExtractRevisionId(newSlice); } - int res = checkRevision(trx, expectedRev, prevRev); + int res = checkRevision(trx, expectedRev, previousMdr.revisionId()); if (res != TRI_ERROR_NO_ERROR) { return Result(res); } } + Result res; // merge old and new values TRI_voc_rid_t revisionId; transaction::BuilderLeaser builder(trx); @@ -3630,11 +3637,10 @@ Result MMFilesCollection::replace(transaction::Methods* trx, VPackSlice const ne if (res.fail()) { operation.revert(trx); } else { - result.setManaged(newDoc.begin(), documentId); - - if (options.waitForSync) { - // store the tick that was used for writing the new document - resultMarkerTick = operation.tick(); + if (options.returnNew) { + resultMdr.setManaged(newDoc.begin()); + } else { // need to pass revId manually + resultMdr.setRevisionId(revisionId); } } @@ -3642,16 +3648,13 @@ Result MMFilesCollection::replace(transaction::Methods* trx, VPackSlice const ne } Result MMFilesCollection::remove(transaction::Methods& trx, velocypack::Slice slice, - ManagedDocumentResult& previous, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, TRI_voc_rid_t& revisionId, - KeyLockInfo* keyLockInfo, - std::function const& callbackDuringLock) { - prevRev = 0; - + ManagedDocumentResult& previousMdr, OperationOptions& options, + bool lock, KeyLockInfo* keyLockInfo, + std::function const& callbackDuringLock) { + LocalDocumentId const documentId = LocalDocumentId::create(); transaction::BuilderLeaser builder(&trx); - + TRI_voc_rid_t revisionId; newObjectForRemove(&trx, slice, *builder.get(), options.isRestore, revisionId); TRI_IF_FAILURE("RemoveDocumentNoMarker") { @@ -3683,7 +3686,6 @@ Result MMFilesCollection::remove(transaction::Methods& trx, velocypack::Slice sl } VPackSlice key; - if (slice.isString()) { key = slice; } else { @@ -3691,6 +3693,9 @@ Result MMFilesCollection::remove(transaction::Methods& trx, velocypack::Slice sl } TRI_ASSERT(!key.isNone()); + if (!key.isString()) { + return Result(TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD); + } TRI_ASSERT(keyLockInfo != nullptr); if (keyLockInfo->shouldLock) { @@ -3705,23 +3710,20 @@ Result MMFilesCollection::remove(transaction::Methods& trx, velocypack::Slice sl trx.state(), lock); // get the previous revision - Result res = lookupDocument(&trx, key, previous); - - if (res.fail()) { - return res; + LocalDocumentId const oldDocumentId = lookupDocument(&trx, key, previousMdr); + if (oldDocumentId.empty()) { + return Result(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND); } - VPackSlice const oldDoc(previous.vpack()); - LocalDocumentId const oldDocumentId = previous.localDocumentId(); - TRI_voc_rid_t oldRevisionId = - arangodb::transaction::helpers::extractRevFromDocument(oldDoc); - prevRev = oldRevisionId; + VPackSlice const oldDoc(previousMdr.vpack()); + TRI_ASSERT(previousMdr.revisionId() != 0); + Result res; // Check old revision: if (!options.ignoreRevs && slice.isObject()) { TRI_voc_rid_t expectedRevisionId = TRI_ExtractRevisionId(slice); - res = checkRevision(&trx, expectedRevisionId, oldRevisionId); + res = checkRevision(&trx, expectedRevisionId, previousMdr.revisionId()); if (res.fail()) { return res; @@ -3767,7 +3769,7 @@ Result MMFilesCollection::remove(transaction::Methods& trx, velocypack::Slice sl ->addOperation(documentId, revisionId, operation, marker, options.waitForSync); if (res.ok() && callbackDuringLock != nullptr) { - res = callbackDuringLock(); + callbackDuringLock(); } } catch (basics::Exception const& ex) { res = Result(ex.code(), ex.what()); @@ -3781,9 +3783,6 @@ Result MMFilesCollection::remove(transaction::Methods& trx, velocypack::Slice sl if (res.fail()) { operation.revert(&trx); - } else { - // store the tick that was used for removing the document - resultMarkerTick = operation.tick(); } return res; } @@ -3950,23 +3949,21 @@ Result MMFilesCollection::removeFastPath(transaction::Methods& trx, TRI_voc_rid_ /// @brief looks up a document by key, low level worker /// the caller must make sure the read lock on the collection is held /// the key must be a string slice, no revision check is performed -Result MMFilesCollection::lookupDocument(transaction::Methods* trx, VPackSlice key, - ManagedDocumentResult& result) { - if (!key.isString()) { - return Result(TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD); - } - +LocalDocumentId MMFilesCollection::lookupDocument(transaction::Methods* trx, + VPackSlice key, + ManagedDocumentResult& result) { + TRI_ASSERT(key.isString()); MMFilesSimpleIndexElement element = primaryIndex()->lookupKey(trx, key, result); if (element) { LocalDocumentId const documentId = element.localDocumentId(); uint8_t const* vpack = lookupDocumentVPack(documentId); if (vpack != nullptr) { - result.setUnmanaged(vpack, documentId); + result.setUnmanaged(vpack); } - return Result(TRI_ERROR_NO_ERROR); + return documentId; } - return Result(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND); + return LocalDocumentId(); } /// @brief updates an existing document, low level worker diff --git a/arangod/MMFiles/MMFilesCollection.h b/arangod/MMFiles/MMFilesCollection.h index 206416758b..6a57595d60 100644 --- a/arangod/MMFiles/MMFilesCollection.h +++ b/arangod/MMFiles/MMFilesCollection.h @@ -301,26 +301,23 @@ class MMFilesCollection final : public PhysicalCollection { TRI_voc_tick_t maxTick, ManagedDocumentResult& result); Result insert(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice newSlice, - arangodb::ManagedDocumentResult& result, - OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool lock, TRI_voc_tick_t& revisionId, KeyLockInfo* keyLockInfo, - std::function const& callbackDuringLock) override; + arangodb::ManagedDocumentResult& resultMdr, + OperationOptions& options, + bool lock, KeyLockInfo* keyLockInfo, + std::function const& callbackDuringLock) override; Result update(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice newSlice, - ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& prevRev, - ManagedDocumentResult& previous) override; + ManagedDocumentResult& resultMdr, OperationOptions& options, + bool lock, ManagedDocumentResult& previousMdr) override; Result replace(transaction::Methods* trx, arangodb::velocypack::Slice newSlice, - ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) override; + ManagedDocumentResult& resultMdr, OperationOptions& options, + bool lock, ManagedDocumentResult& previousMdr) override; Result remove(transaction::Methods& trx, velocypack::Slice slice, - ManagedDocumentResult& previous, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& prevRev, - TRI_voc_rid_t& revisionId, KeyLockInfo* keyLockInfo, - std::function const& callbackDuringLock) override; + ManagedDocumentResult& previousMdr, OperationOptions& options, + bool lock, KeyLockInfo* keyLockInfo, + std::function const& callbackDuringLock) override; Result rollbackOperation(transaction::Methods& trx, TRI_voc_document_operation_e type, LocalDocumentId const& oldDocumentId, @@ -445,8 +442,8 @@ class MMFilesCollection final : public PhysicalCollection { Result deleteSecondaryIndexes(transaction::Methods& trx, LocalDocumentId const& documentId, velocypack::Slice const& doc, Index::OperationMode mode); - Result lookupDocument(transaction::Methods*, velocypack::Slice, - ManagedDocumentResult& result); + LocalDocumentId lookupDocument(transaction::Methods*, velocypack::Slice, + ManagedDocumentResult& result); Result updateDocument(transaction::Methods& trx, TRI_voc_rid_t revisionId, LocalDocumentId const& oldDocumentId, diff --git a/arangod/MMFiles/MMFilesDocumentOperation.cpp b/arangod/MMFiles/MMFilesDocumentOperation.cpp index f7c6a08495..f0cbcb525e 100644 --- a/arangod/MMFiles/MMFilesDocumentOperation.cpp +++ b/arangod/MMFiles/MMFilesDocumentOperation.cpp @@ -35,13 +35,12 @@ using namespace arangodb; MMFilesDocumentOperation::MMFilesDocumentOperation(LogicalCollection* collection, TRI_voc_document_operation_e type) - : _collection(collection), _tick(0), _type(type), _status(StatusType::CREATED) {} + : _collection(collection), _type(type), _status(StatusType::CREATED) {} MMFilesDocumentOperation::~MMFilesDocumentOperation() {} MMFilesDocumentOperation* MMFilesDocumentOperation::clone() { MMFilesDocumentOperation* copy = new MMFilesDocumentOperation(_collection, _type); - copy->_tick = _tick; copy->_oldRevision = _oldRevision; copy->_newRevision = _newRevision; copy->_status = _status; diff --git a/arangod/MMFiles/MMFilesDocumentOperation.h b/arangod/MMFiles/MMFilesDocumentOperation.h index b5f893a7b3..56cb6ef11e 100644 --- a/arangod/MMFiles/MMFilesDocumentOperation.h +++ b/arangod/MMFiles/MMFilesDocumentOperation.h @@ -76,9 +76,6 @@ struct MMFilesDocumentOperation { void setVPack(uint8_t const* vpack); - void setTick(TRI_voc_tick_t tick) { _tick = tick; } - TRI_voc_tick_t tick() const { return _tick; } - TRI_voc_document_operation_e type() const { return _type; } LogicalCollection* collection() const { return _collection; } @@ -101,7 +98,6 @@ struct MMFilesDocumentOperation { LogicalCollection* _collection; MMFilesDocumentDescriptor _oldRevision; MMFilesDocumentDescriptor _newRevision; - TRI_voc_tick_t _tick; TRI_voc_document_operation_e _type; StatusType _status; }; diff --git a/arangod/MMFiles/MMFilesIndexLookupContext.cpp b/arangod/MMFiles/MMFilesIndexLookupContext.cpp index ee7d408656..06238dc114 100644 --- a/arangod/MMFiles/MMFilesIndexLookupContext.cpp +++ b/arangod/MMFiles/MMFilesIndexLookupContext.cpp @@ -40,7 +40,7 @@ uint8_t const* MMFilesIndexLookupContext::lookup(LocalDocumentId token) const { try { uint8_t const* vpack = static_cast(_collection->getPhysical())->lookupDocumentVPack(token); if (vpack != nullptr && _result != nullptr) { - _result->setUnmanaged(vpack, token); + _result->setUnmanaged(vpack); } return vpack; } catch (...) { diff --git a/arangod/MMFiles/MMFilesTransactionState.cpp b/arangod/MMFiles/MMFilesTransactionState.cpp index 48122643d5..9649c4dd1f 100644 --- a/arangod/MMFiles/MMFilesTransactionState.cpp +++ b/arangod/MMFiles/MMFilesTransactionState.cpp @@ -33,6 +33,8 @@ #include "MMFiles/MMFilesLogfileManager.h" #include "MMFiles/MMFilesPersistentIndexFeature.h" #include "MMFiles/MMFilesTransactionCollection.h" +#include "StorageEngine/EngineSelectorFeature.h" +#include "StorageEngine/StorageEngine.h" #include "StorageEngine/TransactionCollection.h" #include "Transaction/Methods.h" #include "VocBase/LogicalCollection.h" @@ -56,7 +58,8 @@ MMFilesTransactionState::MMFilesTransactionState(TRI_vocbase_t& vocbase, TRI_voc : TransactionState(vocbase, tid, options), _rocksTransaction(nullptr), _beginWritten(false), - _hasOperations(false) {} + _hasOperations(false), + _lastWrittenOperationTick(0) {} /// @brief free a transaction container MMFilesTransactionState::~MMFilesTransactionState() { @@ -285,7 +288,8 @@ int MMFilesTransactionState::addOperation(LocalDocumentId const& documentId, } } - operation.setTick(slotInfo.tick); + _lastWrittenOperationTick = slotInfo.tick; + fid = slotInfo.logfileId; position = slotInfo.mem; } else { @@ -469,6 +473,10 @@ int MMFilesTransactionState::writeAbortMarker() { /// @brief write WAL commit marker int MMFilesTransactionState::writeCommitMarker() { if (!needWriteMarker(false)) { + if (_options.waitForSync && _lastWrittenOperationTick > 0 && + isSingleOperation()) { // we do the waitForSync in the end + EngineSelectorFeature::ENGINE->waitForSyncTick(_lastWrittenOperationTick); + } return TRI_ERROR_NO_ERROR; } diff --git a/arangod/MMFiles/MMFilesTransactionState.h b/arangod/MMFiles/MMFilesTransactionState.h index 105eacec3b..a9bef7000c 100644 --- a/arangod/MMFiles/MMFilesTransactionState.h +++ b/arangod/MMFiles/MMFilesTransactionState.h @@ -96,10 +96,15 @@ class MMFilesTransactionState final : public TransactionState { /// @brief free all operations for a transaction void freeOperations(transaction::Methods* activeTrx); + + private: rocksdb::Transaction* _rocksTransaction; bool _beginWritten; bool _hasOperations; + + /// @brief tick of last added & written operation + TRI_voc_tick_t _lastWrittenOperationTick; }; } // namespace arangodb diff --git a/arangod/Replication/utilities.cpp b/arangod/Replication/utilities.cpp index b135163ece..ca96f51b40 100644 --- a/arangod/Replication/utilities.cpp +++ b/arangod/Replication/utilities.cpp @@ -569,7 +569,7 @@ Result buildHttpError(httpclient::SimpleHttpResult* response, if (response == nullptr || !response->isComplete()) { std::string errorMsg; - connection.lease([&errorMsg, &response](httpclient::SimpleHttpClient* client) { + connection.lease([&errorMsg](httpclient::SimpleHttpClient* client) { errorMsg = client->getErrorMessage(); }); if (errorMsg.empty() && response != nullptr) { diff --git a/arangod/RocksDBEngine/RocksDBCollection.cpp b/arangod/RocksDBEngine/RocksDBCollection.cpp index 09e28871b0..b61ff47cd0 100644 --- a/arangod/RocksDBEngine/RocksDBCollection.cpp +++ b/arangod/RocksDBEngine/RocksDBCollection.cpp @@ -392,16 +392,16 @@ std::shared_ptr RocksDBCollection::createIndex(VPackSlice const& info, RocksDBKey key; // read collection info from database key.constructCollection(_logicalCollection.vocbase().id(), _logicalCollection.id()); - rocksdb::PinnableSlice value; + rocksdb::PinnableSlice ps; rocksdb::Status s = engine->db()->Get(rocksdb::ReadOptions(), RocksDBColumnFamily::definitions(), - key.string(), &value); + key.string(), &ps); if (!s.ok()) { res.reset(rocksutils::convertStatus(s)); } else { VPackBuilder builder; builder.openObject(); - for (auto const& pair : VPackObjectIterator(VPackSlice(value.data()))) { + for (auto const& pair : VPackObjectIterator(VPackSlice(ps.data()))) { if (pair.key.isEqualString("indexes")) { // append new index VPackArrayBuilder arrGuard(&builder, "indexes"); builder.add(VPackArrayIterator(pair.value)); @@ -790,10 +790,20 @@ Result RocksDBCollection::read(transaction::Methods* trx, ManagedDocumentResult& result, bool /*lock*/) { LocalDocumentId const documentId = primaryIndex()->lookupKey(trx, key); if (!documentId.isSet()) { - return Result(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND); + return TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND; + } // found + + std::string* buffer = result.setManaged(); + rocksdb::PinnableSlice ps(buffer); + Result res = lookupDocumentVPack(trx, documentId, ps, /*readCache*/true, /*fillCache*/true); + if (res.ok()) { + if (ps.IsPinned()) { + buffer->assign(ps.data(), ps.size()); + } // else value is already assigned + result.setRevisionId(); // extracts id from buffer } - // found - return lookupDocumentVPack(documentId, trx, result, /*withCache*/true); + + return res; } // read using a token! @@ -801,8 +811,15 @@ bool RocksDBCollection::readDocument(transaction::Methods* trx, LocalDocumentId const& documentId, ManagedDocumentResult& result) const { if (documentId.isSet()) { - auto res = lookupDocumentVPack(documentId, trx, result, /*withCache*/true); - return res.ok(); + std::string* buffer = result.setManaged(); + rocksdb::PinnableSlice ps(buffer); + Result res = lookupDocumentVPack(trx, documentId, ps, /*readCache*/true, /*fillCache*/true); + if (res.ok()) { + if (ps.IsPinned()) { + buffer->assign(ps.data(), ps.size()); + } // else value is already assigned + return true; + } } return false; } @@ -812,33 +829,29 @@ bool RocksDBCollection::readDocumentWithCallback(transaction::Methods* trx, LocalDocumentId const& documentId, IndexIterator::DocumentCallback const& cb) const { if (documentId.isSet()) { - return lookupDocumentVPack(documentId, trx, cb, true).ok(); + return lookupDocumentVPack(trx, documentId, cb, /*withCache*/true); } return false; } Result RocksDBCollection::insert(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const slice, - arangodb::ManagedDocumentResult& mdr, - OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool, TRI_voc_tick_t& revisionId, KeyLockInfo* /*keyLockInfo*/, - std::function const& callbackDuringLock) { - // store the tick that was used for writing the document - // note that we don't need it for this engine - resultMarkerTick = 0; + arangodb::ManagedDocumentResult& resultMdr, + OperationOptions& options, + bool /*lock*/, KeyLockInfo* /*keyLockInfo*/, + std::function const& cbDuringLock) { bool const isEdgeCollection = (TRI_COL_TYPE_EDGE == _logicalCollection.type()); transaction::BuilderLeaser builder(trx); + TRI_voc_tick_t revisionId; Result res(newObjectForInsert(trx, slice, isEdgeCollection, *builder.get(), options.isRestore, revisionId)); - if (res.fail()) { return res; } VPackSlice newSlice = builder->slice(); - if (options.overwrite) { // special optimization for the overwrite case: // in case the operation is a RepSert, we will first check if the specified @@ -879,25 +892,27 @@ Result RocksDBCollection::insert(arangodb::transaction::Methods* trx, if (res.ok()) { trackWaitForSync(trx, options); - if (options.silent) { - mdr.clear(); - } else { - mdr.setManaged(newSlice.begin(), documentId); - TRI_ASSERT(!mdr.empty()); + + if (options.returnNew) { + resultMdr.setManaged(newSlice.begin()); + TRI_ASSERT(resultMdr.revisionId() == revisionId); + } else if(!options.silent) { // need to pass revId manually + transaction::BuilderLeaser keyBuilder(trx); + keyBuilder->openObject(/*unindexed*/true); + keyBuilder->add(StaticStrings::KeyString, transaction::helpers::extractKeyFromDocument(newSlice)); + keyBuilder->close(); + resultMdr.setManaged()->assign(reinterpret_cast(keyBuilder->start()), + keyBuilder->size()); + resultMdr.setRevisionId(revisionId); } - + bool hasPerformedIntermediateCommit = false; + res = state->addOperation(_logicalCollection.id(), revisionId, + TRI_VOC_DOCUMENT_OPERATION_INSERT, + hasPerformedIntermediateCommit); - auto result = state->addOperation(_logicalCollection.id(), revisionId, - TRI_VOC_DOCUMENT_OPERATION_INSERT, - hasPerformedIntermediateCommit); - - if (result.ok() && callbackDuringLock != nullptr) { - result = callbackDuringLock(); - } - - if (result.fail()) { - THROW_ARANGO_EXCEPTION(result); + if (res.ok() && cbDuringLock != nullptr) { + cbDuringLock(); } guard.finish(hasPerformedIntermediateCommit); @@ -908,50 +923,46 @@ Result RocksDBCollection::insert(arangodb::transaction::Methods* trx, Result RocksDBCollection::update(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, - ManagedDocumentResult& mdr, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool /*lock*/, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) { - resultMarkerTick = 0; + ManagedDocumentResult& resultMdr, OperationOptions& options, + bool /*lock*/, ManagedDocumentResult& previousMdr) { - VPackSlice key = newSlice.get(StaticStrings::KeyString); - if (key.isNone()) { - return Result(TRI_ERROR_ARANGO_DOCUMENT_HANDLE_BAD); + VPackSlice keySlice = newSlice.get(StaticStrings::KeyString); + if (keySlice.isNone()) { + return TRI_ERROR_ARANGO_DOCUMENT_HANDLE_BAD; + } else if (!keySlice.isString()) { + return TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD; } - - LocalDocumentId const newDocumentId = LocalDocumentId::create(); - auto isEdgeCollection = (TRI_COL_TYPE_EDGE == _logicalCollection.type()); - Result res = this->read(trx, key, previous, /*lock*/ false); + auto const oldDocumentId = primaryIndex()->lookupKey(trx, VPackStringRef(keySlice)); + if (!oldDocumentId.isSet()) { + return TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND; + } + std::string* prevBuffer = previousMdr.setManaged(); + // uses either prevBuffer or avoids memcpy (if read hits block cache) + rocksdb::PinnableSlice previousPS(prevBuffer); + Result res = lookupDocumentVPack(trx, oldDocumentId, previousPS, + /*readCache*/true, /*fillCache*/false); if (res.fail()) { return res; } - TRI_ASSERT(!previous.empty()); - LocalDocumentId const oldDocumentId = previous.localDocumentId(); - VPackSlice oldDoc(previous.vpack()); - TRI_voc_rid_t const oldRevisionId = transaction::helpers::extractRevFromDocument(oldDoc); - - prevRev = oldRevisionId; - - // Check old revision: - if (!options.ignoreRevs) { - TRI_voc_rid_t expectedRev = 0; - - if (newSlice.isObject()) { - expectedRev = TRI_ExtractRevisionId(newSlice); - } - - int result = checkRevision(trx, expectedRev, prevRev); + TRI_ASSERT(previousPS.size() > 0); + VPackSlice const oldDoc(previousPS.data()); + previousMdr.setRevisionId(transaction::helpers::extractRevFromDocument(oldDoc)); + TRI_ASSERT(previousMdr.revisionId() != 0); + if (!options.ignoreRevs) { // Check old revision: + TRI_voc_rid_t expectedRev = TRI_ExtractRevisionId(newSlice); + int result = checkRevision(trx, expectedRev, previousMdr.revisionId()); if (result != TRI_ERROR_NO_ERROR) { return res.reset(result); } } - if (newSlice.length() <= 1) { + if (newSlice.length() <= 1) { // TODO move above ?! // shortcut. no need to do anything - mdr = previous; - TRI_ASSERT(!mdr.empty()); + resultMdr.setManaged(oldDoc.begin()); + TRI_ASSERT(!resultMdr.empty()); trackWaitForSync(trx, options); return res; @@ -959,11 +970,13 @@ Result RocksDBCollection::update(arangodb::transaction::Methods* trx, // merge old and new values TRI_voc_rid_t revisionId; + LocalDocumentId const newDocumentId = LocalDocumentId::create(); + auto isEdgeCollection = (TRI_COL_TYPE_EDGE == _logicalCollection.type()); + transaction::BuilderLeaser builder(trx); res = mergeObjectsForUpdate(trx, oldDoc, newSlice, isEdgeCollection, options.mergeObjects, options.keepNull, *builder.get(), options.isRestore, revisionId); - if (res.fail()) { return res; } @@ -974,15 +987,14 @@ Result RocksDBCollection::update(arangodb::transaction::Methods* trx, return res.reset(TRI_ERROR_CLUSTER_MUST_NOT_CHANGE_SHARDING_ATTRIBUTES); } if (arangodb::smartJoinAttributeChanged(_logicalCollection, oldDoc, builder->slice(), true)) { - return Result(TRI_ERROR_CLUSTER_MUST_NOT_CHANGE_SMART_JOIN_ATTRIBUTE); + return res.reset(TRI_ERROR_CLUSTER_MUST_NOT_CHANGE_SMART_JOIN_ATTRIBUTE); } } VPackSlice const newDoc(builder->slice()); - - auto* state = RocksDBTransactionState::toState(trx); RocksDBSavePoint guard(trx, TRI_VOC_DOCUMENT_OPERATION_UPDATE); + auto* state = RocksDBTransactionState::toState(trx); // add possible log statement under guard state->prepareOperation(_logicalCollection.id(), revisionId, TRI_VOC_DOCUMENT_OPERATION_UPDATE); res = updateDocument(trx, oldDocumentId, oldDoc, newDocumentId, newDoc, options); @@ -990,15 +1002,22 @@ Result RocksDBCollection::update(arangodb::transaction::Methods* trx, if (res.ok()) { trackWaitForSync(trx, options); - if (options.silent) { - mdr.clear(); + if (options.returnNew) { + resultMdr.setManaged(newDoc.begin()); + TRI_ASSERT(!resultMdr.empty()); + } else { // need to pass revId manually + resultMdr.setRevisionId(revisionId); + } + if (options.returnOld) { + if (previousPS.IsPinned()) { // value was not copied + prevBuffer->assign(previousPS.data(), previousPS.size()); + } // else value is already assigned + TRI_ASSERT(!previousMdr.empty()); } else { - mdr.setManaged(newDoc.begin(), newDocumentId); - TRI_ASSERT(!mdr.empty()); + previousMdr.clearData(); } bool hasPerformedIntermediateCommit = false; - auto result = state->addOperation(_logicalCollection.id(), revisionId, TRI_VOC_DOCUMENT_OPERATION_UPDATE, hasPerformedIntermediateCommit); @@ -1014,42 +1033,37 @@ Result RocksDBCollection::update(arangodb::transaction::Methods* trx, Result RocksDBCollection::replace(transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, - ManagedDocumentResult& mdr, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) { - resultMarkerTick = 0; - - // get the previous revision - VPackSlice key = newSlice.get(StaticStrings::KeyString); - Result res; - if (key.isNone()) { - return res.reset(TRI_ERROR_ARANGO_DOCUMENT_HANDLE_BAD); + ManagedDocumentResult& resultMdr, OperationOptions& options, + bool /*lock*/, ManagedDocumentResult& previousMdr) { + + VPackSlice keySlice = newSlice.get(StaticStrings::KeyString); + if (keySlice.isNone()) { + return TRI_ERROR_ARANGO_DOCUMENT_HANDLE_BAD; + } else if (!keySlice.isString()) { + return TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD; } - LocalDocumentId const newDocumentId = LocalDocumentId::create(); - bool const isEdgeCollection = (TRI_COL_TYPE_EDGE == _logicalCollection.type()); - - // get the previous revision - res = this->read(trx, key, previous, /*lock*/ false); + auto const oldDocumentId = primaryIndex()->lookupKey(trx, VPackStringRef(keySlice)); + if (!oldDocumentId.isSet()) { + return TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND; + } + std::string* prevBuffer = previousMdr.setManaged(); + // uses either prevBuffer or avoids memcpy (if read hits block cache) + rocksdb::PinnableSlice previousPS(prevBuffer); + Result res = lookupDocumentVPack(trx, oldDocumentId, previousPS, + /*readCache*/true, /*fillCache*/false); if (res.fail()) { return res; } + + TRI_ASSERT(previousPS.size() > 0); + VPackSlice const oldDoc(previousPS.data()); + previousMdr.setRevisionId(transaction::helpers::extractRevFromDocument(oldDoc)); + TRI_ASSERT(previousMdr.revisionId() != 0); - TRI_ASSERT(!previous.empty()); - LocalDocumentId const oldDocumentId = previous.localDocumentId(); - - VPackSlice oldDoc(previous.vpack()); - TRI_voc_rid_t oldRevisionId = transaction::helpers::extractRevFromDocument(oldDoc); - prevRev = oldRevisionId; - - // Check old revision: - if (!options.ignoreRevs) { - TRI_voc_rid_t expectedRev = 0; - if (newSlice.isObject()) { - expectedRev = TRI_ExtractRevisionId(newSlice); - } - - res = checkRevision(trx, expectedRev, prevRev); + if (!options.ignoreRevs) { // Check old revision: + TRI_voc_rid_t expectedRev = TRI_ExtractRevisionId(newSlice); + res = checkRevision(trx, expectedRev, previousMdr.revisionId()); if (res.fail()) { return res; } @@ -1057,10 +1071,12 @@ Result RocksDBCollection::replace(transaction::Methods* trx, // merge old and new values TRI_voc_rid_t revisionId; + LocalDocumentId const newDocumentId = LocalDocumentId::create(); + bool const isEdgeCollection = (TRI_COL_TYPE_EDGE == _logicalCollection.type()); + transaction::BuilderLeaser builder(trx); res = newObjectForReplace(trx, oldDoc, newSlice, isEdgeCollection, *builder.get(), options.isRestore, revisionId); - if (res.fail()) { return res; } @@ -1076,27 +1092,32 @@ Result RocksDBCollection::replace(transaction::Methods* trx, } VPackSlice const newDoc(builder->slice()); - - auto* state = RocksDBTransactionState::toState(trx); RocksDBSavePoint guard(trx, TRI_VOC_DOCUMENT_OPERATION_REPLACE); + auto* state = RocksDBTransactionState::toState(trx); // add possible log statement under guard state->prepareOperation(_logicalCollection.id(), revisionId, TRI_VOC_DOCUMENT_OPERATION_REPLACE); - res = updateDocument(trx, oldDocumentId, oldDoc, newDocumentId, newDoc, options); if (res.ok()) { trackWaitForSync(trx, options); - if (options.silent) { - mdr.clear(); + if (options.returnNew) { + resultMdr.setManaged(newDoc.begin()); + TRI_ASSERT(!resultMdr.empty()); + } else { // need to pass revId manually + resultMdr.setRevisionId(revisionId); + } + if (options.returnOld) { + if (previousPS.IsPinned()) { // value was not copied + prevBuffer->assign(previousPS.data(), previousPS.size()); + } // else value is already assigned + TRI_ASSERT(!previousMdr.empty()); } else { - mdr.setManaged(newDoc.begin(), newDocumentId); - TRI_ASSERT(!mdr.empty()); + previousMdr.clearData(); } bool hasPerformedIntermediateCommit = false; - auto result = state->addOperation(_logicalCollection.id(), revisionId, TRI_VOC_DOCUMENT_OPERATION_REPLACE, hasPerformedIntermediateCommit); @@ -1112,44 +1133,43 @@ Result RocksDBCollection::replace(transaction::Methods* trx, } Result RocksDBCollection::remove(transaction::Methods& trx, velocypack::Slice slice, - ManagedDocumentResult& previous, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool /*lock*/, - TRI_voc_rid_t& prevRev, TRI_voc_rid_t& revisionId, - KeyLockInfo* /*keyLockInfo*/, - std::function const& callbackDuringLock) { - // store the tick that was used for writing the document - // note that we don't need it for this engine - resultMarkerTick = 0; - prevRev = 0; - revisionId = newRevisionId(); + ManagedDocumentResult& previousMdr, OperationOptions& options, + bool /*lock*/, KeyLockInfo* /*keyLockInfo*/, + std::function const& cbDuringLock) { - VPackSlice key; + VPackSlice keySlice; if (slice.isString()) { - key = slice; + keySlice = slice; } else { - key = slice.get(StaticStrings::KeyString); + keySlice = slice.get(StaticStrings::KeyString); } - TRI_ASSERT(!key.isNone()); - - // get the previous revision - Result res = this->read(&trx, key, previous, /*lock*/ false); - + TRI_ASSERT(!keySlice.isNone()); + if (!keySlice.isString()) { + return TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD; + } + + auto const documentId = primaryIndex()->lookupKey(&trx, VPackStringRef(keySlice)); + if (!documentId.isSet()) { + return TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND; + } + std::string* prevBuffer = previousMdr.setManaged(); + // uses either prevBuffer or avoids memcpy (if read hits block cache) + rocksdb::PinnableSlice previousPS(prevBuffer); + Result res = lookupDocumentVPack(&trx, documentId, previousPS, + /*readCache*/true, /*fillCache*/false); if (res.fail()) { return res; } - - TRI_ASSERT(!previous.empty()); - LocalDocumentId const oldDocumentId = previous.localDocumentId(); - - VPackSlice oldDoc(previous.vpack()); - TRI_voc_rid_t oldRevisionId = - arangodb::transaction::helpers::extractRevFromDocument(oldDoc); - prevRev = oldRevisionId; + + TRI_ASSERT(previousPS.size() > 0); + VPackSlice const oldDoc(previousPS.data()); + previousMdr.setRevisionId(transaction::helpers::extractRevFromDocument(oldDoc)); + TRI_ASSERT(previousMdr.revisionId() != 0); // Check old revision: if (!options.ignoreRevs && slice.isObject()) { TRI_voc_rid_t expectedRevisionId = TRI_ExtractRevisionId(slice); - res = checkRevision(&trx, expectedRevisionId, oldRevisionId); + res = checkRevision(&trx, expectedRevisionId, previousMdr.revisionId()); if (res.fail()) { return res; @@ -1160,24 +1180,28 @@ Result RocksDBCollection::remove(transaction::Methods& trx, velocypack::Slice sl RocksDBSavePoint guard(&trx, TRI_VOC_DOCUMENT_OPERATION_REMOVE); // add possible log statement under guard - state->prepareOperation(_logicalCollection.id(), oldRevisionId, + state->prepareOperation(_logicalCollection.id(), previousMdr.revisionId(), TRI_VOC_DOCUMENT_OPERATION_REMOVE); - res = removeDocument(&trx, oldDocumentId, oldDoc, options); + res = removeDocument(&trx, documentId, oldDoc, options); if (res.ok()) { trackWaitForSync(&trx, options); + if (options.returnOld) { + if (previousPS.IsPinned()) { // value was not copied + prevBuffer->assign(previousPS.data(), previousPS.size()); + } // else value is already assigned + TRI_ASSERT(!previousMdr.empty()); + } else { + previousMdr.clearData(); + } + bool hasPerformedIntermediateCommit = false; - - res = state->addOperation(_logicalCollection.id(), revisionId, TRI_VOC_DOCUMENT_OPERATION_REMOVE, + res = state->addOperation(_logicalCollection.id(), newRevisionId(), TRI_VOC_DOCUMENT_OPERATION_REMOVE, hasPerformedIntermediateCommit); - if (res.ok() && callbackDuringLock != nullptr) { - res = callbackDuringLock(); - } - - if (res.fail()) { - THROW_ARANGO_EXCEPTION(res); + if (res.ok() && cbDuringLock != nullptr) { + cbDuringLock(); } guard.finish(hasPerformedIntermediateCommit); @@ -1355,10 +1379,14 @@ Result RocksDBCollection::updateDocument(transaction::Methods* trx, return res; } -arangodb::Result RocksDBCollection::lookupDocumentVPack(LocalDocumentId const& documentId, - transaction::Methods* trx, - arangodb::ManagedDocumentResult& mdr, - bool withCache) const { +/// @brief lookup document in cache and / or rocksdb +/// @param readCache attempt to read from cache +/// @param fillCache fill cache with found document +arangodb::Result RocksDBCollection::lookupDocumentVPack(transaction::Methods* trx, + LocalDocumentId const& documentId, + rocksdb::PinnableSlice& ps, + bool readCache, + bool fillCache) const { TRI_ASSERT(trx->state()->isRunning()); TRI_ASSERT(_objectId != 0); Result res; @@ -1367,15 +1395,15 @@ arangodb::Result RocksDBCollection::lookupDocumentVPack(LocalDocumentId const& d key->constructDocument(_objectId, documentId); bool lockTimeout = false; - if (withCache && useCache()) { + if (readCache && useCache()) { TRI_ASSERT(_cache != nullptr); // check cache first for fast path auto f = _cache->find(key->string().data(), static_cast(key->string().size())); - if (f.found()) { - std::string* value = mdr.setManaged(documentId); - value->append(reinterpret_cast(f.value()->value()), - f.value()->valueSize()); + if (f.found()) { // copy finding into buffer + ps.PinSelf(rocksdb::Slice(reinterpret_cast(f.value()->value()), + f.value()->valueSize())); + // TODO we could potentially use the PinSlice method ?! return res; } if (f.result().errorNumber() == TRI_ERROR_LOCK_TIMEOUT) { @@ -1384,64 +1412,56 @@ arangodb::Result RocksDBCollection::lookupDocumentVPack(LocalDocumentId const& d lockTimeout = true; // we skip the insert in this case } } - + RocksDBMethods* mthd = RocksDBTransactionState::toMethods(trx); - std::string* value = mdr.setManaged(documentId); - rocksdb::Status s = mthd->Get(RocksDBColumnFamily::documents(), key->string(), value); - - if (s.ok()) { - if (withCache && useCache() && !lockTimeout) { - TRI_ASSERT(_cache != nullptr); - // write entry back to cache - auto entry = - cache::CachedValue::construct(key->string().data(), - static_cast(key->string().size()), - value->data(), - static_cast(value->size())); - - if (entry) { - Result status = _cache->insert(entry); - if (status.errorNumber() == TRI_ERROR_LOCK_TIMEOUT) { - // the writeLock uses cpu_relax internally, so we can try yield - std::this_thread::yield(); - status = _cache->insert(entry); - } - - if (status.fail()) { - delete entry; - } + rocksdb::Status s = mthd->Get(RocksDBColumnFamily::documents(), key->string(), &ps); + + if (!s.ok()) { + LOG_TOPIC("f63dd", DEBUG, Logger::ENGINES) + << "NOT FOUND rev: " << documentId.id() << " trx: " << trx->state()->id() + << " objectID " << _objectId << " name: " << _logicalCollection.name(); + return res.reset(rocksutils::convertStatus(s, rocksutils::document)); + } + + if (fillCache && useCache() && !lockTimeout) { + TRI_ASSERT(_cache != nullptr); + // write entry back to cache + auto entry = + cache::CachedValue::construct(key->string().data(), + static_cast(key->string().size()), + ps.data(), static_cast(ps.size())); + if (entry) { + auto status = _cache->insert(entry); + if (status.errorNumber() == TRI_ERROR_LOCK_TIMEOUT) { + // the writeLock uses cpu_relax internally, so we can try yield + std::this_thread::yield(); + status = _cache->insert(entry); + } + if (status.fail()) { + delete entry; } } - } else { - LOG_TOPIC("bd363", DEBUG, Logger::ENGINES) - << "NOT FOUND rev: " << documentId.id() << " trx: " << trx->state()->id() - << " objectID " << _objectId << " name: " << _logicalCollection.name(); - mdr.clear(); - res.reset(rocksutils::convertStatus(s, rocksutils::document)); } - + return res; } -arangodb::Result RocksDBCollection::lookupDocumentVPack( - LocalDocumentId const& documentId, transaction::Methods* trx, - IndexIterator::DocumentCallback const& cb, bool withCache) const { - TRI_ASSERT(trx->state()->isRunning()); - TRI_ASSERT(_objectId != 0); - Result res; - - RocksDBKeyLeaser key(trx); - key->constructDocument(_objectId, documentId); +bool RocksDBCollection::lookupDocumentVPack(transaction::Methods* trx, + LocalDocumentId const& documentId, + IndexIterator::DocumentCallback const& cb, + bool withCache) const { bool lockTimeout = false; if (withCache && useCache()) { + RocksDBKeyLeaser key(trx); + key->constructDocument(_objectId, documentId); TRI_ASSERT(_cache != nullptr); // check cache first for fast path auto f = _cache->find(key->string().data(), static_cast(key->string().size())); if (f.found()) { cb(documentId, VPackSlice(reinterpret_cast(f.value()->value()))); - return res; + return true; } if (f.result().errorNumber() == TRI_ERROR_LOCK_TIMEOUT) { // assuming someone is currently holding a write lock, which @@ -1449,40 +1469,16 @@ arangodb::Result RocksDBCollection::lookupDocumentVPack( lockTimeout = true; // we skip the insert in this case } } - - rocksdb::PinnableSlice ps; - RocksDBMethods* mthd = RocksDBTransactionState::toMethods(trx); - rocksdb::Status s = mthd->Get(RocksDBColumnFamily::documents(), key->string(), &ps); - - if (s.ok()) { - if (withCache && useCache() && !lockTimeout) { - TRI_ASSERT(_cache != nullptr); - // write entry back to cache - auto entry = - cache::CachedValue::construct(key->string().data(), - static_cast(key->string().size()), - ps.data(), static_cast(ps.size())); - if (entry) { - auto status = _cache->insert(entry); - if (status.errorNumber() == TRI_ERROR_LOCK_TIMEOUT) { - // the writeLock uses cpu_relax internally, so we can try yield - std::this_thread::yield(); - status = _cache->insert(entry); - } - if (status.fail()) { - delete entry; - } - } - } - + + transaction::StringLeaser buffer(trx); + rocksdb::PinnableSlice ps(buffer.get()); + Result res = lookupDocumentVPack(trx, documentId, ps, /*readCache*/false, withCache); + if (res.ok()) { + TRI_ASSERT(ps.size() > 0); cb(documentId, VPackSlice(ps.data())); - } else { - LOG_TOPIC("f63dd", DEBUG, Logger::ENGINES) - << "NOT FOUND rev: " << documentId.id() << " trx: " << trx->state()->id() - << " objectID " << _objectId << " name: " << _logicalCollection.name(); - res.reset(rocksutils::convertStatus(s, rocksutils::document)); + return true; } - return res; + return false; } /// may never be called unless recovery is finished diff --git a/arangod/RocksDBEngine/RocksDBCollection.h b/arangod/RocksDBEngine/RocksDBCollection.h index 28c2c17b05..e2a19bba04 100644 --- a/arangod/RocksDBEngine/RocksDBCollection.h +++ b/arangod/RocksDBEngine/RocksDBCollection.h @@ -1,7 +1,7 @@ //////////////////////////////////////////////////////////////////////////////// /// DISCLAIMER /// -/// Copyright 2014-2017 ArangoDB GmbH, Cologne, Germany +/// Copyright 2014-2019 ArangoDB GmbH, Cologne, Germany /// Copyright 2004-2014 triAGENS GmbH, Cologne, Germany /// /// Licensed under the Apache License, Version 2.0 (the "License"); @@ -32,6 +32,7 @@ #include "VocBase/LogicalCollection.h" namespace rocksdb { +class PinnableSlice; class Transaction; } @@ -134,30 +135,27 @@ class RocksDBCollection final : public PhysicalCollection { bool readDocument(transaction::Methods* trx, LocalDocumentId const& token, ManagedDocumentResult& result) const override; + /// @brief lookup with callback, not thread-safe on same transaction::Context bool readDocumentWithCallback(transaction::Methods* trx, LocalDocumentId const& token, IndexIterator::DocumentCallback const& cb) const override; Result insert(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice newSlice, - arangodb::ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_tick_t& revisionId, KeyLockInfo* /*keyLockInfo*/, - std::function const& callbackDuringLock) override; + arangodb::ManagedDocumentResult& resultMdr, OperationOptions& options, + bool lock, KeyLockInfo* /*keyLockInfo*/, + std::function const& cbDuringLock) override; Result update(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice newSlice, - ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& prevRev, - ManagedDocumentResult& previous) override; + ManagedDocumentResult& resultMdr, OperationOptions& options, + bool lock, ManagedDocumentResult& previousMdr) override; Result replace(transaction::Methods* trx, arangodb::velocypack::Slice newSlice, - ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) override; + ManagedDocumentResult& resultMdr, OperationOptions& options, + bool lock, ManagedDocumentResult& previousMdr) override; Result remove(transaction::Methods& trx, velocypack::Slice slice, ManagedDocumentResult& previous, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& prevRev, - TRI_voc_rid_t& revisionId, KeyLockInfo* keyLockInfo, - std::function const& callbackDuringLock) override; + bool lock, KeyLockInfo* keyLockInfo, + std::function const& cbDuringLock) override; /// adjust the current number of docs void adjustNumberDocuments(TRI_voc_rid_t revisionId, int64_t adjustment); @@ -209,18 +207,23 @@ class RocksDBCollection final : public PhysicalCollection { arangodb::velocypack::Slice const& newDoc, OperationOptions& options) const; - arangodb::Result lookupDocumentVPack(LocalDocumentId const& documentId, - transaction::Methods*, - arangodb::ManagedDocumentResult&, - bool withCache) const; - - arangodb::Result lookupDocumentVPack(LocalDocumentId const& documentId, - transaction::Methods*, - IndexIterator::DocumentCallback const& cb, - bool withCache) const; + /// @brief lookup document in cache and / or rocksdb + /// @param readCache attempt to read from cache + /// @param fillCache fill cache with found document + arangodb::Result lookupDocumentVPack(transaction::Methods* trx, + LocalDocumentId const& documentId, + rocksdb::PinnableSlice& ps, + bool readCache, + bool fillCache) const; + + bool lookupDocumentVPack(transaction::Methods*, + LocalDocumentId const& documentId, + IndexIterator::DocumentCallback const& cb, + bool withCache) const; + /// @brief create hash-cache void createCache() const; - + /// @brief destory hash-cache void destroyCache() const; /// is this collection using a cache diff --git a/arangod/RocksDBEngine/RocksDBEdgeIndex.cpp b/arangod/RocksDBEngine/RocksDBEdgeIndex.cpp index 3385148d02..4663475b8e 100644 --- a/arangod/RocksDBEngine/RocksDBEdgeIndex.cpp +++ b/arangod/RocksDBEngine/RocksDBEdgeIndex.cpp @@ -44,6 +44,7 @@ #include "Transaction/Helpers.h" #include "Transaction/Methods.h" #include "VocBase/LogicalCollection.h" +#include "VocBase/ManagedDocumentResult.h" #include #include @@ -894,6 +895,8 @@ void RocksDBEdgeIndex::warmupInternal(transaction::Methods* trx, rocksdb::Slice options.fill_cache = EdgeIndexFillBlockCache; std::unique_ptr it( rocksutils::globalRocksDB()->NewIterator(options, _cf)); + + ManagedDocumentResult mdr; size_t n = 0; cache::Cache* cc = _cache.get(); @@ -971,21 +974,20 @@ void RocksDBEdgeIndex::warmupInternal(transaction::Methods* trx, rocksdb::Slice } if (needsInsert) { LocalDocumentId const docId = - RocksDBKey::indexDocumentId(RocksDBEntryType::EdgeIndexValue, key); - if (!rocksColl->readDocumentWithCallback(trx, docId, [&](LocalDocumentId const&, VPackSlice doc) { - builder.add(VPackValue(docId.id())); - VPackSlice toFrom = - _isFromIndex ? transaction::helpers::extractToFromDocument(doc) - : transaction::helpers::extractFromFromDocument(doc); - TRI_ASSERT(toFrom.isString()); - builder.add(toFrom); - })) { -#ifdef ARANGODB_ENABLE_MAINTAINER_MODE - // Data Inconsistency. - // We have a revision id without a document... + RocksDBKey::indexDocumentId(RocksDBEntryType::EdgeIndexValue, key); + if (!rocksColl->readDocument(trx, docId, mdr)) { + // Data Inconsistency. revision id without a document... TRI_ASSERT(false); -#endif + continue; } + + builder.add(VPackValue(docId.id())); + VPackSlice doc(mdr.vpack()); + VPackSlice toFrom = + _isFromIndex ? transaction::helpers::extractToFromDocument(doc) + : transaction::helpers::extractFromFromDocument(doc); + TRI_ASSERT(toFrom.isString()); + builder.add(toFrom); } } diff --git a/arangod/RocksDBEngine/RocksDBIncrementalSync.cpp b/arangod/RocksDBEngine/RocksDBIncrementalSync.cpp index 842b681b50..4b43bdbd8e 100644 --- a/arangod/RocksDBEngine/RocksDBIncrementalSync.cpp +++ b/arangod/RocksDBEngine/RocksDBIncrementalSync.cpp @@ -96,8 +96,6 @@ Result removeKeysOutsideRange(VPackSlice chunkSlice, LogicalCollection* coll, VPackBuilder builder; ManagedDocumentResult mdr; - TRI_voc_tick_t tick; - TRI_voc_rid_t prevRev, revisionId; // remove everything from the beginning of the key range until the lowest // remote key @@ -108,8 +106,8 @@ Result removeKeysOutsideRange(VPackSlice chunkSlice, LogicalCollection* coll, builder.clear(); builder.add(velocypack::ValuePair(docKey.data(), docKey.size(), velocypack::ValueType::String)); - auto r = physical->remove(trx, builder.slice(), mdr, options, tick, - false, prevRev, revisionId, nullptr, nullptr); + auto r = physical->remove(trx, builder.slice(), mdr, options, + /*lock*/false, nullptr, nullptr); if (r.fail() && r.isNot(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND)) { // ignore not found, we remove conflicting docs ahead of time @@ -145,8 +143,8 @@ Result removeKeysOutsideRange(VPackSlice chunkSlice, LogicalCollection* coll, builder.clear(); builder.add(velocypack::ValuePair(docKey.data(), docKey.size(), velocypack::ValueType::String)); - auto r = physical->remove(trx, builder.slice(), mdr, options, tick, - false, prevRev, revisionId, nullptr, nullptr); + auto r = physical->remove(trx, builder.slice(), mdr, options, + /*lock*/false, nullptr, nullptr); if (r.fail() && r.isNot(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND)) { // ignore not found, we remove conflicting docs ahead of time @@ -252,8 +250,6 @@ Result syncChunkRocksDB(DatabaseInitialSyncer& syncer, SingleCollectionTransacti // state for RocksDBCollection insert/replace/remove ManagedDocumentResult mdr, previous; - TRI_voc_tick_t resultTick; - TRI_voc_rid_t prevRev, revisionId; transaction::BuilderLeaser keyBuilder(trx); std::vector toFetch; @@ -297,8 +293,8 @@ Result syncChunkRocksDB(DatabaseInitialSyncer& syncer, SingleCollectionTransacti keyBuilder->clear(); keyBuilder->add(VPackValue(localKey)); - auto r = physical->remove(*trx, keyBuilder->slice(), mdr, options, resultTick, - false, prevRev, revisionId, nullptr, nullptr); + auto r = physical->remove(*trx, keyBuilder->slice(), mdr, options, + /*lock*/false, nullptr, nullptr); if (r.fail() && r.isNot(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND)) { // ignore not found, we remove conflicting docs ahead of time @@ -351,8 +347,8 @@ Result syncChunkRocksDB(DatabaseInitialSyncer& syncer, SingleCollectionTransacti keyBuilder->clear(); keyBuilder->add(VPackValue(localKey)); - auto r = physical->remove(*trx, keyBuilder->slice(), mdr, options, resultTick, - false, prevRev, revisionId, nullptr, nullptr); + auto r = physical->remove(*trx, keyBuilder->slice(), mdr, options, + /*lock*/false, nullptr, nullptr); if (r.fail() && r.isNot(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND)) { // ignore not found, we remove conflicting docs ahead of time @@ -476,8 +472,8 @@ Result syncChunkRocksDB(DatabaseInitialSyncer& syncer, SingleCollectionTransacti keyBuilder->clear(); keyBuilder->add(VPackValue(conflictingKey)); - auto res = physical->remove(*trx, keyBuilder->slice(), mdr, options, resultTick, - false, prevRev, revisionId, nullptr, nullptr); + auto res = physical->remove(*trx, keyBuilder->slice(), mdr, options, + /*lock*/false, nullptr, nullptr); if (res.ok()) { ++stats.numDocsRemoved; @@ -492,8 +488,8 @@ Result syncChunkRocksDB(DatabaseInitialSyncer& syncer, SingleCollectionTransacti // INSERT TRI_ASSERT(options.indexOperationMode == Index::OperationMode::internal); - Result res = physical->insert(trx, it, mdr, options, resultTick, false, - revisionId, nullptr, nullptr); + Result res = physical->insert(trx, it, mdr, options, + /*lock*/false, nullptr, nullptr); if (res.fail()) { if (res.is(TRI_ERROR_ARANGO_UNIQUE_CONSTRAINT_VIOLATED) && res.errorMessage() > keySlice.copyString()) { @@ -504,8 +500,8 @@ Result syncChunkRocksDB(DatabaseInitialSyncer& syncer, SingleCollectionTransacti return res; } - res = physical->insert(trx, it, mdr, options, resultTick, false, - revisionId, nullptr, nullptr); + res = physical->insert(trx, it, mdr, options, + /*lock*/false, nullptr, nullptr); if (res.fail()) { return res; } @@ -520,8 +516,8 @@ Result syncChunkRocksDB(DatabaseInitialSyncer& syncer, SingleCollectionTransacti // REPLACE TRI_ASSERT(options.indexOperationMode == Index::OperationMode::internal); - Result res = physical->replace(trx, it, mdr, options, resultTick, false, - prevRev, previous); + Result res = physical->replace(trx, it, mdr, options, + /*lock*/false, previous); if (res.fail()) { if (res.is(TRI_ERROR_ARANGO_UNIQUE_CONSTRAINT_VIOLATED) && res.errorMessage() > keySlice.copyString()) { @@ -531,8 +527,8 @@ Result syncChunkRocksDB(DatabaseInitialSyncer& syncer, SingleCollectionTransacti if (inner.fail()) { return res; } - res = physical->replace(trx, it, mdr, options, resultTick, false, - prevRev, previous); + res = physical->replace(trx, it, mdr, options, + /*lock*/false, previous); if (res.fail()) { return res; } @@ -735,11 +731,8 @@ Result handleSyncKeysRocksDB(DatabaseInitialSyncer& syncer, tempBuilder.add(VPackValue(docKey)); ManagedDocumentResult previous; - TRI_voc_rid_t resultMarkerTick; - TRI_voc_rid_t prevRev, revisionId; auto r = physical->remove(trx, tempBuilder.slice(), previous, - options, resultMarkerTick, false, prevRev, - revisionId, nullptr, nullptr); + options, /*lock*/false, nullptr, nullptr); if (r.fail() && r.isNot(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND)) { // ignore not found, we remove conflicting docs ahead of time diff --git a/arangod/RocksDBEngine/RocksDBPrimaryIndex.h b/arangod/RocksDBEngine/RocksDBPrimaryIndex.h index 95082f8483..9d0ee3698b 100644 --- a/arangod/RocksDBEngine/RocksDBPrimaryIndex.h +++ b/arangod/RocksDBEngine/RocksDBPrimaryIndex.h @@ -76,7 +76,8 @@ class RocksDBPrimaryIndex final : public RocksDBIndex { void toVelocyPack(VPackBuilder&, std::underlying_type::type) const override; - LocalDocumentId lookupKey(transaction::Methods* trx, arangodb::velocypack::StringRef key) const; + LocalDocumentId lookupKey(transaction::Methods* trx, + arangodb::velocypack::StringRef key) const; /// @brief reads a revision id from the primary index /// if the document does not exist, this function will return false diff --git a/arangod/RocksDBEngine/RocksDBVPackIndex.cpp b/arangod/RocksDBEngine/RocksDBVPackIndex.cpp index b323d23589..b31118a92f 100644 --- a/arangod/RocksDBEngine/RocksDBVPackIndex.cpp +++ b/arangod/RocksDBEngine/RocksDBVPackIndex.cpp @@ -111,12 +111,12 @@ class RocksDBVPackUniqueIndexIterator final : public IndexIterator { _done = true; - rocksdb::PinnableSlice val; + rocksdb::PinnableSlice ps; RocksDBMethods* mthds = RocksDBTransactionState::toMethods(_trx); - rocksdb::Status s = mthds->Get(_index->columnFamily(), _key->string(), &val); + rocksdb::Status s = mthds->Get(_index->columnFamily(), _key->string(), &ps); if (s.ok()) { - cb(RocksDBValue::documentId(val)); + cb(RocksDBValue::documentId(ps)); } // there is at most one element, so we are done now @@ -133,12 +133,12 @@ class RocksDBVPackUniqueIndexIterator final : public IndexIterator { _done = true; - rocksdb::PinnableSlice val; + rocksdb::PinnableSlice ps; RocksDBMethods* mthds = RocksDBTransactionState::toMethods(_trx); - rocksdb::Status s = mthds->Get(_index->columnFamily(), _key->string(), &val); + rocksdb::Status s = mthds->Get(_index->columnFamily(), _key->string(), &ps); if (s.ok()) { - cb(LocalDocumentId(RocksDBValue::documentId(val)), + cb(LocalDocumentId(RocksDBValue::documentId(ps)), RocksDBKey::indexedVPack(_key.ref())); } diff --git a/arangod/StorageEngine/PhysicalCollection.h b/arangod/StorageEngine/PhysicalCollection.h index 0477b15fce..3610cb987d 100644 --- a/arangod/StorageEngine/PhysicalCollection.h +++ b/arangod/StorageEngine/PhysicalCollection.h @@ -156,48 +156,47 @@ class PhysicalCollection { virtual Result read(transaction::Methods*, arangodb::velocypack::Slice const& key, ManagedDocumentResult& result, bool lock) = 0; + /// @brief read a documument referenced by token (internal method) virtual bool readDocument(transaction::Methods* trx, LocalDocumentId const& token, ManagedDocumentResult& result) const = 0; + /// @brief read a documument referenced by token (internal method) virtual bool readDocumentWithCallback(transaction::Methods* trx, LocalDocumentId const& token, IndexIterator::DocumentCallback const& cb) const = 0; /** - * @param callbackDuringLock Called immediately after a successful insert. If - * it returns a failure, the insert will be rolled back. If the insert wasn't - * successful, it isn't called. May be nullptr. + * @brief Perform document insert, may generate a '_key' value + * If (options.returnNew == false && !options.silent) result might + * just contain an object with the '_key' field + * @param callbackDuringLock Called immediately after a successful insert. + * If the insert wasn't successful, it isn't called. May be nullptr. */ virtual Result insert(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice newSlice, arangodb::ManagedDocumentResult& result, - OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool lock, TRI_voc_tick_t& revisionId, KeyLockInfo* keyLockInfo, - std::function const& callbackDuringLock) = 0; + OperationOptions& options, + bool lock, KeyLockInfo* keyLockInfo, + std::function const& cbDuringLock) = 0; Result insert(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice newSlice, - arangodb::ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock) { - TRI_voc_rid_t unused; - return insert(trx, newSlice, result, options, resultMarkerTick, lock, - unused, nullptr, nullptr); + arangodb::ManagedDocumentResult& result, OperationOptions& options, bool lock) { + return insert(trx, newSlice, result, options, lock, nullptr, nullptr); } virtual Result update(arangodb::transaction::Methods* trx, arangodb::velocypack::Slice newSlice, ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) = 0; + bool lock, ManagedDocumentResult& previous) = 0; - virtual Result replace(transaction::Methods* trx, arangodb::velocypack::Slice newSlice, + virtual Result replace(arangodb::transaction::Methods* trx, + arangodb::velocypack::Slice newSlice, ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) = 0; + bool lock, ManagedDocumentResult& previous) = 0; virtual Result remove(transaction::Methods& trx, velocypack::Slice slice, ManagedDocumentResult& previous, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& prevRev, - TRI_voc_rid_t& revisionId, KeyLockInfo* keyLockInfo, - std::function const& callbackDuringLock) = 0; + bool lock, KeyLockInfo* keyLockInfo, + std::function const& cbDuringLock) = 0; protected: PhysicalCollection(LogicalCollection& collection, arangodb::velocypack::Slice const& info); diff --git a/arangod/Transaction/Context.cpp b/arangod/Transaction/Context.cpp index fb9f4fb06a..34dfa09a94 100644 --- a/arangod/Transaction/Context.cpp +++ b/arangod/Transaction/Context.cpp @@ -69,6 +69,7 @@ transaction::Context::Context(TRI_vocbase_t& vocbase) _customTypeHandler(), _builders{_arena}, _stringBuffer(), + _strings{_strArena}, _options(arangodb::velocypack::Options::Defaults), _dumpOptions(arangodb::velocypack::Options::Defaults), _transaction{0, false}, @@ -143,17 +144,26 @@ void transaction::Context::returnStringBuffer(basics::StringBuffer* stringBuffer /// @brief temporarily lease a std::string std::string* transaction::Context::leaseString() { - if (_stdString == nullptr) { - _stdString.reset(new std::string()); - } else { - _stdString->clear(); + if (_strings.empty()) { + // create a new string and return it + return new std::string(); } - return _stdString.release(); + + // re-use an existing string + std::string* s = _strings.back(); + s->clear(); + _strings.pop_back(); + return s; } /// @brief return a temporary std::string object void transaction::Context::returnString(std::string* str) { - _stdString.reset(str); + try { // put string back into our vector of strings + _strings.push_back(str); + } catch (...) { + // no harm done. just wipe the string + delete str; + } } /// @brief temporarily lease a Builder object diff --git a/arangod/Transaction/Context.h b/arangod/Transaction/Context.h index 4e8eab49ea..72f68b41c0 100644 --- a/arangod/Transaction/Context.h +++ b/arangod/Transaction/Context.h @@ -141,15 +141,15 @@ class Context { SmallVector::allocator_type::arena_type _arena; SmallVector _builders; - + std::unique_ptr _stringBuffer; - std::unique_ptr _stdString; + + SmallVector::allocator_type::arena_type _strArena; + SmallVector _strings; arangodb::velocypack::Options _options; arangodb::velocypack::Options _dumpOptions; -private: - private: std::unique_ptr _contextData; diff --git a/arangod/Transaction/Methods.cpp b/arangod/Transaction/Methods.cpp index 80a2c8ce35..932fd2f021 100644 --- a/arangod/Transaction/Methods.cpp +++ b/arangod/Transaction/Methods.cpp @@ -1748,16 +1748,14 @@ OperationResult transaction::Methods::insertLocal(std::string const& collectionN std::shared_ptr const> followers; - std::function updateFollowers = nullptr; + std::function updateFollowers; if (needsToGetFollowersUnderLock) { FollowerInfo const& followerInfo = *collection->followers(); - updateFollowers = [&followerInfo, &followers]() -> Result { + updateFollowers = [&followerInfo, &followers]() { TRI_ASSERT(followers == nullptr); followers = followerInfo.get(); - - return Result{}; }; } else if (_state->isDBServer()) { TRI_ASSERT(followers == nullptr); @@ -1808,8 +1806,8 @@ OperationResult transaction::Methods::insertLocal(std::string const& collectionN } VPackBuilder resultBuilder; - ManagedDocumentResult documentResult; - TRI_voc_tick_t maxTick = 0; + ManagedDocumentResult docResult; + ManagedDocumentResult prevDocResult; // return OLD (with override option) auto workForOneDocument = [&](VPackSlice const value) -> Result { if (!value.isObject()) { @@ -1822,9 +1820,8 @@ OperationResult transaction::Methods::insertLocal(std::string const& collectionN return Result(r); } - TRI_voc_tick_t resultMarkerTick = 0; - TRI_voc_rid_t revisionId = 0; - documentResult.clear(); + docResult.clear(); + prevDocResult.clear(); // insert with overwrite may NOT be a single document operation, as we // possibly need to do two separate operations (insert and replace). @@ -1832,34 +1829,21 @@ OperationResult transaction::Methods::insertLocal(std::string const& collectionN TRI_ASSERT(needsLock == !isLocked(collection, AccessMode::Type::WRITE)); Result res = - collection->insert(this, value, documentResult, options, resultMarkerTick, - needsLock, revisionId, &keyLockInfo, updateFollowers); - - TRI_voc_rid_t previousRevisionId = 0; - ManagedDocumentResult previousDocumentResult; // return OLD + collection->insert(this, value, docResult, options, + needsLock, &keyLockInfo, updateFollowers); + bool didReplace = false; if (options.overwrite && res.is(TRI_ERROR_ARANGO_UNIQUE_CONSTRAINT_VIOLATED)) { - // RepSert Case - unique_constraint violated -> maxTick has not changed -> - // try replace - resultMarkerTick = 0; + // RepSert Case - unique_constraint violated -> try replace // If we're overwriting, we already have a lock. Therefore we also don't // need to get the followers under the lock. TRI_ASSERT(!needsLock); TRI_ASSERT(!needsToGetFollowersUnderLock); TRI_ASSERT(updateFollowers == nullptr); - res = collection->replace(this, value, documentResult, options, - resultMarkerTick, false, previousRevisionId, - previousDocumentResult); - if (res.ok() && !options.silent) { - // If we are silent, then revisionId will not be looked at further - // down. In the silent case, documentResult is empty, so nobody - // must actually look at it! - revisionId = TRI_ExtractRevisionId(VPackSlice(documentResult.vpack())); - } - } - - if (resultMarkerTick > 0 && resultMarkerTick > maxTick) { - maxTick = resultMarkerTick; + res = collection->replace(this, value, docResult, options, + /*lock*/false, prevDocResult); + TRI_ASSERT(res.fail() || prevDocResult.revisionId() != 0); + didReplace = true; } if (res.fail()) { @@ -1869,21 +1853,22 @@ OperationResult transaction::Methods::insertLocal(std::string const& collectionN } if (!options.silent) { - TRI_ASSERT(!documentResult.empty()); + const bool showReplaced = (options.returnOld && didReplace); + TRI_ASSERT(!options.returnNew || !docResult.empty()); + TRI_ASSERT(!showReplaced || !prevDocResult.empty()); - arangodb::velocypack::StringRef keyString(transaction::helpers::extractKeyFromDocument( - VPackSlice(documentResult.vpack()))); - - bool showReplaced = false; - if (options.returnOld && previousRevisionId) { - showReplaced = true; - TRI_ASSERT(!previousDocumentResult.empty()); + arangodb::velocypack::StringRef keyString; + if (didReplace) { // docResult may be empty, but replace requires '_key' in value + keyString = value.get(StaticStrings::KeyString); + TRI_ASSERT(!keyString.empty()); + } else { + keyString = transaction::helpers::extractKeyFromDocument(VPackSlice(docResult.vpack())); } - + buildDocumentIdentity(collection, resultBuilder, cid, keyString, - revisionId, previousRevisionId, - showReplaced ? &previousDocumentResult : nullptr, - options.returnNew ? &documentResult : nullptr); + docResult.revisionId(), prevDocResult.revisionId(), + showReplaced ? &prevDocResult : nullptr, + options.returnNew ? &docResult : nullptr); } return Result(); }; @@ -1921,11 +1906,6 @@ OperationResult transaction::Methods::insertLocal(std::string const& collectionN } } - // wait for operation(s) to be synced to disk here. On rocksdb maxTick == 0 - if (res.ok() && options.waitForSync && maxTick > 0 && isSingleOperationTransaction()) { - EngineSelectorFeature::ENGINE->waitForSyncTick(maxTick); - } - if (options.silent) { // We needed the results, but do not want to report: resultBuilder.clear(); @@ -2130,12 +2110,11 @@ OperationResult transaction::Methods::modifyLocal(std::string const& collectionN TRI_ASSERT(needsLock == lockResult.is(TRI_ERROR_LOCKED)); VPackBuilder resultBuilder; // building the complete result - TRI_voc_tick_t maxTick = 0; ManagedDocumentResult previous; ManagedDocumentResult result; // lambda ////////////// - auto workForOneDocument = [this, &operation, &options, &maxTick, &collection, + auto workForOneDocument = [this, &operation, &options, &collection, &resultBuilder, &cid, &previous, &result](VPackSlice const newVal, bool isBabies) -> Result { Result res; @@ -2144,8 +2123,6 @@ OperationResult transaction::Methods::modifyLocal(std::string const& collectionN return res; } - TRI_voc_rid_t actualRevision = 0; - TRI_voc_tick_t resultMarkerTick = 0; result.clear(); previous.clear(); @@ -2154,33 +2131,32 @@ OperationResult transaction::Methods::modifyLocal(std::string const& collectionN TRI_ASSERT(isLocked(collection, AccessMode::Type::WRITE)); if (operation == TRI_VOC_DOCUMENT_OPERATION_REPLACE) { - res = collection->replace(this, newVal, result, options, resultMarkerTick, - false, actualRevision, previous); + res = collection->replace(this, newVal, result, options, + /*lock*/false, previous); } else { - res = collection->update(this, newVal, result, options, resultMarkerTick, - false, actualRevision, previous); - } - - if (resultMarkerTick > 0 && resultMarkerTick > maxTick) { - maxTick = resultMarkerTick; + res = collection->update(this, newVal, result, options, + /*lock*/false, previous); } if (res.fail()) { if (res.is(TRI_ERROR_ARANGO_CONFLICT) && !isBabies) { + TRI_ASSERT(previous.revisionId() != 0); arangodb::velocypack::StringRef key(newVal.get(StaticStrings::KeyString)); - buildDocumentIdentity(collection, resultBuilder, cid, key, actualRevision, + buildDocumentIdentity(collection, resultBuilder, cid, key, previous.revisionId(), 0, options.returnOld ? &previous : nullptr, nullptr); } return res; } if (!options.silent) { - TRI_ASSERT(!previous.empty()); - TRI_ASSERT(!result.empty()); + TRI_ASSERT(!options.returnOld || !previous.empty()); + TRI_ASSERT(!options.returnNew || !result.empty()); + TRI_ASSERT(result.revisionId() != 0 && previous.revisionId() != 0); + arangodb::velocypack::StringRef key(newVal.get(StaticStrings::KeyString)); buildDocumentIdentity(collection, resultBuilder, cid, key, - TRI_ExtractRevisionId(VPackSlice(result.vpack())), - actualRevision, options.returnOld ? &previous : nullptr, + result.revisionId(), previous.revisionId(), + options.returnOld ? &previous : nullptr, options.returnNew ? &result : nullptr); } @@ -2234,11 +2210,6 @@ OperationResult transaction::Methods::modifyLocal(std::string const& collectionN } } - // wait for operation(s) to be synced to disk here. On rocksdb maxTick == 0 - if (res.ok() && options.waitForSync && maxTick > 0 && isSingleOperationTransaction()) { - EngineSelectorFeature::ENGINE->waitForSyncTick(maxTick); - } - if (options.silent) { // We needed the results, but do not want to report: resultBuilder.clear(); @@ -2338,16 +2309,14 @@ OperationResult transaction::Methods::removeLocal(std::string const& collectionN std::shared_ptr const> followers; - std::function updateFollowers = nullptr; + std::function updateFollowers = nullptr; if (needsToGetFollowersUnderLock) { auto const& followerInfo = *collection->followers(); - updateFollowers = [&followerInfo, &followers]() -> Result { + updateFollowers = [&followerInfo, &followers]() { TRI_ASSERT(followers == nullptr); followers = followerInfo.get(); - - return Result{}; }; } else if (_state->isDBServer()) { TRI_ASSERT(followers == nullptr); @@ -2399,10 +2368,8 @@ OperationResult transaction::Methods::removeLocal(std::string const& collectionN VPackBuilder resultBuilder; ManagedDocumentResult previous; - TRI_voc_tick_t maxTick = 0; auto workForOneDocument = [&](VPackSlice value, bool isBabies) -> Result { - TRI_voc_rid_t actualRevision = 0; transaction::BuilderLeaser builder(this); arangodb::velocypack::StringRef key; if (value.isString()) { @@ -2423,33 +2390,30 @@ OperationResult transaction::Methods::removeLocal(std::string const& collectionN return Result(TRI_ERROR_ARANGO_DOCUMENT_HANDLE_BAD); } - TRI_voc_tick_t resultMarkerTick = 0; previous.clear(); TRI_ASSERT(needsLock == !isLocked(collection, AccessMode::Type::WRITE)); - auto res = collection->remove(*this, value, options, resultMarkerTick, needsLock, - actualRevision, previous, &keyLockInfo, updateFollowers); - - if (resultMarkerTick > 0 && resultMarkerTick > maxTick) { - maxTick = resultMarkerTick; - } + auto res = collection->remove(*this, value, options, needsLock, + previous, &keyLockInfo, updateFollowers); if (res.fail()) { if (res.is(TRI_ERROR_ARANGO_CONFLICT) && !isBabies) { - buildDocumentIdentity(collection, resultBuilder, cid, key, actualRevision, + TRI_ASSERT(previous.revisionId() != 0); + buildDocumentIdentity(collection, resultBuilder, cid, key, previous.revisionId(), 0, options.returnOld ? &previous : nullptr, nullptr); } return res; } - TRI_ASSERT(!previous.empty()); if (!options.silent) { - buildDocumentIdentity(collection, resultBuilder, cid, key, actualRevision, + TRI_ASSERT(!options.returnOld || !previous.empty()); + TRI_ASSERT(previous.revisionId() != 0); + buildDocumentIdentity(collection, resultBuilder, cid, key, previous.revisionId(), 0, options.returnOld ? &previous : nullptr, nullptr); } - return Result(); + return res; }; Result res; @@ -2486,11 +2450,6 @@ OperationResult transaction::Methods::removeLocal(std::string const& collectionN } } - // wait for operation(s) to be synced to disk here. On rocksdb maxTick == 0 - if (res.ok() && options.waitForSync && maxTick > 0 && isSingleOperationTransaction()) { - EngineSelectorFeature::ENGINE->waitForSyncTick(maxTick); - } - if (options.silent) { // We needed the results, but do not want to report: resultBuilder.clear(); @@ -3365,7 +3324,7 @@ Result Methods::replicateOperations(LogicalCollection const& collection, std::shared_ptr> const& followers, OperationOptions const& options, VPackSlice const value, TRI_voc_document_operation_e const operation, - VPackBuilder& resultBuilder) { + VPackBuilder const& resultBuilder) { TRI_ASSERT(followers != nullptr); Result res; diff --git a/arangod/Transaction/Methods.h b/arangod/Transaction/Methods.h index a3db51dedb..11dec7849d 100644 --- a/arangod/Transaction/Methods.h +++ b/arangod/Transaction/Methods.h @@ -501,13 +501,6 @@ class Methods { ENTERPRISE_VIRT Result unlockRecursive(TRI_voc_cid_t, AccessMode::Type); private: - /// @brief replicates operations from leader to follower(s) - Result replicateOperations(LogicalCollection* collection, - arangodb::velocypack::Slice const& inputValue, - arangodb::velocypack::Builder const& resultBuilder, - std::shared_ptr const>& followers, - arangodb::rest::RequestType requestType, - std::string const& pathAppendix); /// @brief Helper create a Cluster Communication document OperationResult clusterResultDocument(rest::ResponseCode const& responseCode, @@ -583,7 +576,7 @@ class Methods { std::shared_ptr> const& followers, OperationOptions const& options, VPackSlice value, TRI_voc_document_operation_e operation, - VPackBuilder& resultBuilder); + VPackBuilder const& resultBuilder); }; } // namespace transaction diff --git a/arangod/VocBase/LogicalCollection.cpp b/arangod/VocBase/LogicalCollection.cpp index 6419a5f261..59539b7528 100644 --- a/arangod/VocBase/LogicalCollection.cpp +++ b/arangod/VocBase/LogicalCollection.cpp @@ -940,68 +940,56 @@ Result LogicalCollection::compact() { Result LogicalCollection::insert(transaction::Methods* trx, VPackSlice const slice, ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& revisionId, KeyLockInfo* keyLockInfo, - std::function callbackDuringLock) { + bool lock, KeyLockInfo* keyLockInfo, + std::function const& cbDuringLock) { TRI_IF_FAILURE("LogicalCollection::insert") { return Result(TRI_ERROR_DEBUG); } - resultMarkerTick = 0; - return getPhysical()->insert(trx, slice, result, options, resultMarkerTick, lock, - revisionId, keyLockInfo, std::move(callbackDuringLock)); + return getPhysical()->insert(trx, slice, result, options, lock, + keyLockInfo, cbDuringLock); } /// @brief updates a document or edge in a collection Result LogicalCollection::update(transaction::Methods* trx, VPackSlice const newSlice, ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) { + bool lock, ManagedDocumentResult& previous) { TRI_IF_FAILURE("LogicalCollection::update") { return Result(TRI_ERROR_DEBUG); } - resultMarkerTick = 0; if (!newSlice.isObject()) { return Result(TRI_ERROR_ARANGO_DOCUMENT_TYPE_INVALID); } - prevRev = 0; - return getPhysical()->update(trx, newSlice, result, options, resultMarkerTick, lock, - prevRev, previous); + return getPhysical()->update(trx, newSlice, result, options, lock, + previous); } /// @brief replaces a document or edge in a collection Result LogicalCollection::replace(transaction::Methods* trx, VPackSlice const newSlice, ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous) { + bool lock, ManagedDocumentResult& previous) { TRI_IF_FAILURE("LogicalCollection::replace") { return Result(TRI_ERROR_DEBUG); } - resultMarkerTick = 0; if (!newSlice.isObject()) { return Result(TRI_ERROR_ARANGO_DOCUMENT_TYPE_INVALID); } - prevRev = 0; - return getPhysical()->replace(trx, newSlice, result, options, resultMarkerTick, lock, - prevRev, previous); + return getPhysical()->replace(trx, newSlice, result, options, lock, + previous); } /// @brief removes a document or edge Result LogicalCollection::remove(transaction::Methods& trx, velocypack::Slice const slice, - OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool lock, TRI_voc_rid_t& prevRev, + OperationOptions& options, bool lock, ManagedDocumentResult& previous, KeyLockInfo* keyLockInfo, - std::function callbackDuringLock) { + std::function const& cbDuringLock) { TRI_IF_FAILURE("LogicalCollection::remove") { return Result(TRI_ERROR_DEBUG); } - resultMarkerTick = 0; - TRI_voc_rid_t revisionId = 0; - return getPhysical()->remove(trx, slice, previous, options, resultMarkerTick, - lock, prevRev, revisionId, keyLockInfo, - std::move(callbackDuringLock)); + return getPhysical()->remove(trx, slice, previous, options, + lock, keyLockInfo, cbDuringLock); } bool LogicalCollection::readDocument(transaction::Methods* trx, LocalDocumentId const& token, diff --git a/arangod/VocBase/LogicalCollection.h b/arangod/VocBase/LogicalCollection.h index 9366be4d76..40e968021f 100644 --- a/arangod/VocBase/LogicalCollection.h +++ b/arangod/VocBase/LogicalCollection.h @@ -297,34 +297,31 @@ class LogicalCollection : public LogicalDataSource { // convenience function for downwards-compatibility Result insert(transaction::Methods* trx, velocypack::Slice const slice, ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock) { - TRI_voc_rid_t unused; - return insert(trx, slice, result, options, resultMarkerTick, lock, unused, - nullptr, nullptr); + bool lock) { + return insert(trx, slice, result, options, lock, nullptr, nullptr); } /** - * @param callbackDuringLock Called immediately after a successful insert. If + * @param cbDuringLock Called immediately after a successful insert. If * it returns a failure, the insert will be rolled back. If the insert wasn't * successful, it isn't called. May be nullptr. */ Result insert(transaction::Methods* trx, velocypack::Slice slice, ManagedDocumentResult& result, OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& revisionId, - KeyLockInfo* keyLockInfo, std::function callbackDuringLock); + bool lock, KeyLockInfo* keyLockInfo, std::function const& cbDuringLock); - Result update(transaction::Methods*, velocypack::Slice, - ManagedDocumentResult& result, OperationOptions&, TRI_voc_tick_t&, - bool lock, TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous); + Result update(transaction::Methods*, velocypack::Slice newSlice, + ManagedDocumentResult& result, OperationOptions&, + bool lock, ManagedDocumentResult& previousMdr); - Result replace(transaction::Methods*, velocypack::Slice, - ManagedDocumentResult& result, OperationOptions&, TRI_voc_tick_t&, - bool lock, TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous); + Result replace(transaction::Methods*, velocypack::Slice newSlice, + ManagedDocumentResult& result, OperationOptions&, + bool lock, ManagedDocumentResult& previousMdr); Result remove(transaction::Methods& trx, velocypack::Slice slice, - OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool lock, TRI_voc_rid_t& prevRev, ManagedDocumentResult& previous, - KeyLockInfo* keyLockInfo, std::function callbackDuringLock); + OperationOptions& options, + bool lock, ManagedDocumentResult& previousMdr, + KeyLockInfo* keyLockInfo, std::function const& cbDuringLock); bool readDocument(transaction::Methods* trx, LocalDocumentId const& token, ManagedDocumentResult& result) const; diff --git a/arangod/VocBase/ManagedDocumentResult.cpp b/arangod/VocBase/ManagedDocumentResult.cpp index 67f2605537..3fbf049174 100644 --- a/arangod/VocBase/ManagedDocumentResult.cpp +++ b/arangod/VocBase/ManagedDocumentResult.cpp @@ -23,6 +23,7 @@ #include "ManagedDocumentResult.h" #include "Aql/AqlValue.h" +#include "Transaction/Helpers.h" #include #include @@ -30,35 +31,35 @@ using namespace arangodb; -void ManagedDocumentResult::setUnmanaged(uint8_t const* vpack, - LocalDocumentId const& documentId) { +void ManagedDocumentResult::setUnmanaged(uint8_t const* vpack) { _string.clear(); _vpack = const_cast(vpack); - _localDocumentId = documentId; - _managed = false; + _revisionId = transaction::helpers::extractRevFromDocument(VPackSlice(vpack)); } -void ManagedDocumentResult::setManaged(uint8_t const* vpack, LocalDocumentId const& documentId) { - _string.assign(reinterpret_cast(vpack), VPackSlice(vpack).byteSize()); +void ManagedDocumentResult::setManaged(uint8_t const* vpack) { + VPackSlice const slice(vpack); + _string.assign(slice.startAs(), slice.byteSize()); _vpack = nullptr; - _localDocumentId = documentId; - _managed = true; + _revisionId = transaction::helpers::extractRevFromDocument(slice); +} + +void ManagedDocumentResult::setRevisionId() noexcept { + TRI_ASSERT(!this->empty()); + _revisionId = transaction::helpers::extractRevFromDocument(VPackSlice(this->vpack())); } void ManagedDocumentResult::addToBuilder(velocypack::Builder& builder, bool allowExternals) const { - uint8_t const* vpack; - if (_managed) { - vpack = reinterpret_cast(_string.data()); + TRI_ASSERT(!empty()); + if (_vpack == nullptr) { // managed + TRI_ASSERT(!_string.empty()); + builder.add(VPackSlice(_string.data())); } else { - vpack = _vpack; - } - TRI_ASSERT(vpack != nullptr); - auto slice = velocypack::Slice(vpack); - TRI_ASSERT(!slice.isExternal()); - if (allowExternals && canUseInExternal()) { - builder.addExternal(slice.begin()); - } else { - builder.add(slice); + if (allowExternals) { + builder.addExternal(_vpack); + } else { + builder.add(VPackSlice(_vpack)); + } } } diff --git a/arangod/VocBase/ManagedDocumentResult.h b/arangod/VocBase/ManagedDocumentResult.h index 1c7177950f..2576d30c7a 100644 --- a/arangod/VocBase/ManagedDocumentResult.h +++ b/arangod/VocBase/ManagedDocumentResult.h @@ -35,7 +35,7 @@ class Builder; class ManagedDocumentResult { public: - ManagedDocumentResult() : _vpack(nullptr), _managed(false) {} + ManagedDocumentResult() : _vpack(nullptr), _revisionId(0) {} ManagedDocumentResult(ManagedDocumentResult const& other) = default; ManagedDocumentResult& operator=(ManagedDocumentResult const& other) = default; @@ -43,9 +43,7 @@ class ManagedDocumentResult { ManagedDocumentResult& operator=(ManagedDocumentResult&& other) noexcept { _string = std::move(other._string); _vpack = other._vpack; - _localDocumentId = other._localDocumentId; - _managed = other._managed; - + _revisionId = other._revisionId; other.clear(); return *this; } @@ -53,51 +51,58 @@ class ManagedDocumentResult { ManagedDocumentResult(ManagedDocumentResult&& other) noexcept : _string(std::move(other._string)), _vpack(other._vpack), - _localDocumentId(other._localDocumentId), - _managed(other._managed) { + _revisionId(other._revisionId) { other.clear(); } - void setUnmanaged(uint8_t const* vpack, LocalDocumentId const& documentId); + /// @brief store pointer to a valid document + void setUnmanaged(uint8_t const* vpack); + /// @brief copy in a valid document + void setManaged(uint8_t const* vpack); - void setManaged(uint8_t const* vpack, LocalDocumentId const& documentId); - - std::string* setManaged(LocalDocumentId const& documentId) { + /// @brief access the internal buffer, revisionId must be set manually + std::string* setManaged() noexcept { _string.clear(); _vpack = nullptr; - _localDocumentId = documentId; - _managed = true; + _revisionId = 0; return &_string; } - inline LocalDocumentId localDocumentId() const { return _localDocumentId; } + inline TRI_voc_rid_t revisionId() const noexcept { return _revisionId; } + void setRevisionId(TRI_voc_rid_t rid) noexcept { _revisionId = rid; } + void setRevisionId() noexcept; - void clear() noexcept { + void clearData() noexcept { _string.clear(); _vpack = nullptr; - _localDocumentId.clear(); - _managed = false; } - - inline uint8_t const* vpack() const { - if (_managed) { + + void clear() noexcept { + clearData(); + _revisionId = 0; + } + + inline uint8_t const* vpack() const noexcept { + if (_vpack == nullptr) { return reinterpret_cast(_string.data()); } - TRI_ASSERT(_vpack != nullptr); return _vpack; } - inline bool empty() const { return (!_managed && _vpack == nullptr); } + inline bool empty() const noexcept { + return (_vpack == nullptr && _string.empty()); + } - inline bool canUseInExternal() const { return !_managed; } + inline bool canUseInExternal() const noexcept { + return _vpack != nullptr; + } void addToBuilder(velocypack::Builder& builder, bool allowExternals) const; private: std::string _string; uint8_t* _vpack; - LocalDocumentId _localDocumentId; - bool _managed; + TRI_voc_rid_t _revisionId; }; } // namespace arangodb diff --git a/js/client/modules/@arangodb/test-utils.js b/js/client/modules/@arangodb/test-utils.js index f878f24882..9fa1e2323e 100755 --- a/js/client/modules/@arangodb/test-utils.js +++ b/js/client/modules/@arangodb/test-utils.js @@ -588,7 +588,7 @@ function runThere (options, instanceInfo, file) { let httpOptions = pu.makeAuthorizationHeaders(options); httpOptions.method = 'POST'; - httpOptions.timeout = 1800; + httpOptions.timeout = 2700; if (options.valgrind) { httpOptions.timeout *= 2; @@ -610,7 +610,8 @@ function runThere (options, instanceInfo, file) { (reply.message.search('timeout during read') >= 0 ) || (reply.message.search('Connection closed by remote') >= 0 ) )) { - print(RED + Date() + " request timeout reached, aborting test execution" + RESET); + print(RED + Date() + " request timeout reached (" + reply.message + + "), aborting test execution" + RESET); return { status: false, message: reply.message, diff --git a/tests/IResearch/IResearchQueryJoin-test.cpp b/tests/IResearch/IResearchQueryJoin-test.cpp index facc60677e..365a65cf51 100644 --- a/tests/IResearch/IResearchQueryJoin-test.cpp +++ b/tests/IResearch/IResearchQueryJoin-test.cpp @@ -312,7 +312,7 @@ TEST_CASE("IResearchQueryTestJoinSubquery", "[iresearch][iresearch-query]") { arangodb::ManagedDocumentResult mmdr; for (auto doc : arangodb::velocypack::ArrayIterator(root)) { - auto const res = entities->insert(&trx, doc, mmdr, opt, tick, false); + auto const res = entities->insert(&trx, doc, mmdr, opt, false); CHECK(res.ok()); } } @@ -329,7 +329,7 @@ TEST_CASE("IResearchQueryTestJoinSubquery", "[iresearch][iresearch-query]") { arangodb::ManagedDocumentResult mmdr; for (auto doc : arangodb::velocypack::ArrayIterator(root)) { - auto const res = links->insert(&trx, doc, mmdr, opt, tick, false); + auto const res = links->insert(&trx, doc, mmdr, opt, false); CHECK(res.ok()); } } @@ -499,7 +499,7 @@ TEST_CASE("IResearchQueryTestJoinDuplicateDataSource", "[iresearch][iresearch-qu for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocsView.emplace_back(); - auto const res = collections[i % 2]->insert(&trx, doc, insertedDocsView.back(), opt, tick, false); + auto const res = collections[i % 2]->insert(&trx, doc, insertedDocsView.back(), opt, false); CHECK(res.ok()); ++i; } @@ -519,7 +519,7 @@ TEST_CASE("IResearchQueryTestJoinDuplicateDataSource", "[iresearch][iresearch-qu for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocsCollection.emplace_back(); - auto const res = logicalCollection3->insert(&trx, doc, insertedDocsCollection.back(), opt, tick, false); + auto const res = logicalCollection3->insert(&trx, doc, insertedDocsCollection.back(), opt, false); CHECK(res.ok()); } } @@ -649,7 +649,7 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") { for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocsView.emplace_back(); - auto const res = collections[i % 2]->insert(&trx, doc, insertedDocsView.back(), opt, tick, false); + auto const res = collections[i % 2]->insert(&trx, doc, insertedDocsView.back(), opt, false); CHECK(res.ok()); ++i; } @@ -669,7 +669,7 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") { for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocsCollection.emplace_back(); - auto const res = logicalCollection3->insert(&trx, doc, insertedDocsCollection.back(), opt, tick, false); + auto const res = logicalCollection3->insert(&trx, doc, insertedDocsCollection.back(), opt, false); CHECK(res.ok()); } } @@ -1883,4 +1883,4 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") { // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- \ No newline at end of file +// ----------------------------------------------------------------------------- diff --git a/tests/IResearch/IResearchQueryNumericTerm-test.cpp b/tests/IResearch/IResearchQueryNumericTerm-test.cpp index 8714aed023..09c4364e76 100644 --- a/tests/IResearch/IResearchQueryNumericTerm-test.cpp +++ b/tests/IResearch/IResearchQueryNumericTerm-test.cpp @@ -271,7 +271,7 @@ TEST_CASE("IResearchQueryTestNumericTerm", "[iresearch][iresearch-query]") { for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocs.emplace_back(); - auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, tick, false); + auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, false); CHECK(res.ok()); ++i; } @@ -3139,4 +3139,4 @@ TEST_CASE("IResearchQueryTestNumericTerm", "[iresearch][iresearch-query]") { // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- \ No newline at end of file +// ----------------------------------------------------------------------------- diff --git a/tests/IResearch/IResearchQueryOptimization-test.cpp b/tests/IResearch/IResearchQueryOptimization-test.cpp index b9a7927ffd..5145900891 100644 --- a/tests/IResearch/IResearchQueryOptimization-test.cpp +++ b/tests/IResearch/IResearchQueryOptimization-test.cpp @@ -332,7 +332,7 @@ TEST_CASE("IResearchQueryTestOptimization", "[iresearch][iresearch-query]") { for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocs.emplace_back(); - auto const res = logicalCollection1->insert(&trx, doc, insertedDocs.back(), opt, tick, false); + auto const res = logicalCollection1->insert(&trx, doc, insertedDocs.back(), opt, false); CHECK(res.ok()); } @@ -8514,4 +8514,4 @@ TEST_CASE("IResearchQueryTestOptimization", "[iresearch][iresearch-query]") { // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- \ No newline at end of file +// ----------------------------------------------------------------------------- diff --git a/tests/IResearch/IResearchQueryOptions-test.cpp b/tests/IResearch/IResearchQueryOptions-test.cpp index a911623d19..8e899e9a93 100644 --- a/tests/IResearch/IResearchQueryOptions-test.cpp +++ b/tests/IResearch/IResearchQueryOptions-test.cpp @@ -300,12 +300,12 @@ TEST_CASE("IResearchQueryTestOptionsCollections", "[iresearch][iresearch-query]" for (auto doc : arangodb::velocypack::ArrayIterator(root)) { { insertedDocs.emplace_back(); - auto const res = collections[0]->insert(&trx, doc, insertedDocs.back(), opt, tick, false); + auto const res = collections[0]->insert(&trx, doc, insertedDocs.back(), opt, false); CHECK(res.ok()); } { insertedDocs.emplace_back(); - auto const res = collections[1]->insert(&trx, doc, insertedDocs.back(), opt, tick, false); + auto const res = collections[1]->insert(&trx, doc, insertedDocs.back(), opt, false); CHECK(res.ok()); } ++i; @@ -959,7 +959,7 @@ TEST_CASE("IResearchQueryTestOptionsWaitForSync", "[iresearch][iresearch-query]" for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocs.emplace_back(); - auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, tick, false); + auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, false); CHECK(res.ok()); ++i; } @@ -1113,4 +1113,4 @@ TEST_CASE("IResearchQueryTestOptionsWaitForSync", "[iresearch][iresearch-query]" // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- \ No newline at end of file +// ----------------------------------------------------------------------------- diff --git a/tests/IResearch/IResearchQueryOr-test.cpp b/tests/IResearch/IResearchQueryOr-test.cpp index 3c38bf54ec..3c29da77e1 100644 --- a/tests/IResearch/IResearchQueryOr-test.cpp +++ b/tests/IResearch/IResearchQueryOr-test.cpp @@ -295,7 +295,7 @@ TEST_CASE("IResearchQueryTestOr", "[iresearch][iresearch-query]") { for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocs.emplace_back(); - auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, tick, false); + auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, false); CHECK(res.ok()); ++i; } @@ -716,4 +716,4 @@ TEST_CASE("IResearchQueryTestOr", "[iresearch][iresearch-query]") { // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- \ No newline at end of file +// ----------------------------------------------------------------------------- diff --git a/tests/IResearch/IResearchQueryScorer-test.cpp b/tests/IResearch/IResearchQueryScorer-test.cpp index 4b0efb760b..f986dcc15b 100644 --- a/tests/IResearch/IResearchQueryScorer-test.cpp +++ b/tests/IResearch/IResearchQueryScorer-test.cpp @@ -333,7 +333,7 @@ TEST_CASE("IResearchQueryScorer", "[iresearch][iresearch-query]") { for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocsView.emplace_back(); - auto const res = collections[i % 2]->insert(&trx, doc, insertedDocsView.back(), opt, tick, false); + auto const res = collections[i % 2]->insert(&trx, doc, insertedDocsView.back(), opt, false); CHECK(res.ok()); ++i; } @@ -353,7 +353,7 @@ TEST_CASE("IResearchQueryScorer", "[iresearch][iresearch-query]") { for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocsCollection.emplace_back(); - auto const res = logicalCollection3->insert(&trx, doc, insertedDocsCollection.back(), opt, tick, false); + auto const res = logicalCollection3->insert(&trx, doc, insertedDocsCollection.back(), opt, false); CHECK(res.ok()); } } diff --git a/tests/IResearch/IResearchQuerySelectAll-test.cpp b/tests/IResearch/IResearchQuerySelectAll-test.cpp index bed122ad51..ca56ebfc18 100644 --- a/tests/IResearch/IResearchQuerySelectAll-test.cpp +++ b/tests/IResearch/IResearchQuerySelectAll-test.cpp @@ -247,14 +247,14 @@ TEST_CASE("IResearchQueryTestSelectAll", "[iresearch][iresearch-query]") { // insert into collection_1 for (; i < insertedDocs.size()/2; ++i) { auto const doc = arangodb::velocypack::Parser::fromJson("{ \"key\": " + std::to_string(i) + "}"); - auto const res = logicalCollection1->insert(&trx, doc->slice(), insertedDocs[i], opt, tick, false); + auto const res = logicalCollection1->insert(&trx, doc->slice(), insertedDocs[i], opt, false); CHECK(res.ok()); } // insert into collection_2 for (; i < insertedDocs.size(); ++i) { auto const doc = arangodb::velocypack::Parser::fromJson("{ \"key\": " + std::to_string(i) + "}"); - auto const res = logicalCollection1->insert(&trx, doc->slice(), insertedDocs[i], opt, tick, false); + auto const res = logicalCollection1->insert(&trx, doc->slice(), insertedDocs[i], opt, false); CHECK(res.ok()); } @@ -508,4 +508,4 @@ TEST_CASE("IResearchQueryTestSelectAll", "[iresearch][iresearch-query]") { // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- \ No newline at end of file +// ----------------------------------------------------------------------------- diff --git a/tests/IResearch/IResearchQueryStartsWith-test.cpp b/tests/IResearch/IResearchQueryStartsWith-test.cpp index 3ddb18e26e..726e374f92 100644 --- a/tests/IResearch/IResearchQueryStartsWith-test.cpp +++ b/tests/IResearch/IResearchQueryStartsWith-test.cpp @@ -260,7 +260,7 @@ TEST_CASE("IResearchQueryTestStartsWith", "[iresearch][iresearch-query]") { for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocs.emplace_back(); - auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, tick, false); + auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, false); CHECK(res.ok()); ++i; } @@ -450,4 +450,4 @@ TEST_CASE("IResearchQueryTestStartsWith", "[iresearch][iresearch-query]") { // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- \ No newline at end of file +// ----------------------------------------------------------------------------- diff --git a/tests/IResearch/IResearchQueryStringTerm-test.cpp b/tests/IResearch/IResearchQueryStringTerm-test.cpp index 46104e5804..734b16d8b6 100644 --- a/tests/IResearch/IResearchQueryStringTerm-test.cpp +++ b/tests/IResearch/IResearchQueryStringTerm-test.cpp @@ -301,7 +301,7 @@ TEST_CASE("IResearchQueryTestStringTerm", "[iresearch][iresearch-query]") { for (auto doc : arangodb::velocypack::ArrayIterator(root)) { insertedDocs.emplace_back(); - auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, tick, false); + auto const res = collections[i % 2]->insert(&trx, doc, insertedDocs.back(), opt, false); CHECK(res.ok()); ++i; } @@ -2905,4 +2905,4 @@ TEST_CASE("IResearchQueryTestStringTerm", "[iresearch][iresearch-query]") { // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE -// ----------------------------------------------------------------------------- \ No newline at end of file +// ----------------------------------------------------------------------------- diff --git a/tests/IResearch/IResearchView-test.cpp b/tests/IResearch/IResearchView-test.cpp index 4df4e3f861..8add2ef038 100644 --- a/tests/IResearch/IResearchView-test.cpp +++ b/tests/IResearch/IResearchView-test.cpp @@ -2123,7 +2123,7 @@ SECTION("test_query") { arangodb::OperationOptions options; for (size_t i = 1; i <= 12; ++i) { auto doc = arangodb::velocypack::Parser::fromJson(std::string("{ \"key\": ") + std::to_string(i) + " }"); - logicalCollection->insert(&trx, doc->slice(), inserted, options, tick, false); + logicalCollection->insert(&trx, doc->slice(), inserted, options, false); } CHECK((trx.commit().ok())); @@ -2156,7 +2156,7 @@ SECTION("test_query") { arangodb::OperationOptions options; for (size_t i = 13; i <= 24; ++i) { auto doc = arangodb::velocypack::Parser::fromJson(std::string("{ \"key\": ") + std::to_string(i) + " }"); - logicalCollection->insert(&trx, doc->slice(), inserted, options, tick, false); + logicalCollection->insert(&trx, doc->slice(), inserted, options, false); } CHECK(trx.commit().ok()); diff --git a/tests/IResearch/IResearchViewDBServer-test.cpp b/tests/IResearch/IResearchViewDBServer-test.cpp index fe9869ffac..a892453f66 100644 --- a/tests/IResearch/IResearchViewDBServer-test.cpp +++ b/tests/IResearch/IResearchViewDBServer-test.cpp @@ -656,7 +656,7 @@ SECTION("test_query") { arangodb::OperationOptions options; for (size_t i = 1; i <= 12; ++i) { auto doc = arangodb::velocypack::Parser::fromJson(std::string("{ \"key\": ") + std::to_string(i) + " }"); - logicalCollection->insert(&trx, doc->slice(), inserted, options, tick, false); + logicalCollection->insert(&trx, doc->slice(), inserted, options, false); } CHECK((trx.commit().ok())); @@ -696,7 +696,7 @@ SECTION("test_query") { arangodb::OperationOptions options; for (size_t i = 13; i <= 24; ++i) { auto doc = arangodb::velocypack::Parser::fromJson(std::string("{ \"key\": ") + std::to_string(i) + " }"); - logicalCollection->insert(&trx, doc->slice(), inserted, options, tick, false); + logicalCollection->insert(&trx, doc->slice(), inserted, options, false); } CHECK(trx.commit().ok()); diff --git a/tests/IResearch/IResearchViewNode-test.cpp b/tests/IResearch/IResearchViewNode-test.cpp index 619ecb8dca..ef07519849 100644 --- a/tests/IResearch/IResearchViewNode-test.cpp +++ b/tests/IResearch/IResearchViewNode-test.cpp @@ -1249,7 +1249,7 @@ TEST_CASE("IResearchViewNodeTest", "[iresearch][iresearch-view-node]") { CHECK((trx.begin().ok())); auto json = arangodb::velocypack::Parser::fromJson("{}"); - auto const res = collection0->insert(&trx, json->slice(), mmdoc, opt, tick, false); + auto const res = collection0->insert(&trx, json->slice(), mmdoc, opt, false); CHECK(res.ok()); CHECK((trx.commit().ok())); diff --git a/tests/Mocks/StorageEngineMock.cpp b/tests/Mocks/StorageEngineMock.cpp index 2ae932a942..dfa39e70f7 100644 --- a/tests/Mocks/StorageEngineMock.cpp +++ b/tests/Mocks/StorageEngineMock.cpp @@ -586,9 +586,8 @@ void PhysicalCollectionMock::getPropertiesVPack(arangodb::velocypack::Builder&) arangodb::Result PhysicalCollectionMock::insert( arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, arangodb::ManagedDocumentResult& result, arangodb::OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_tick_t& revisionId, - arangodb::KeyLockInfo* /*keyLockInfo*/, - std::function const& callbackDuringLock) { + bool lock, arangodb::KeyLockInfo* /*keyLockInfo*/, + std::function const& callbackDuringLock) { TRI_ASSERT(callbackDuringLock == nullptr); // not implemented before(); @@ -606,7 +605,8 @@ arangodb::Result PhysicalCollectionMock::insert( documents.emplace_back(std::move(builder), true); arangodb::LocalDocumentId docId(documents.size()); // always > 0 - result.setUnmanaged(documents.back().first.data(), docId); + result.setUnmanaged(documents.back().first.data()); + TRI_ASSERT(result.revisionId() == unused); for (auto& index : _indexes) { if (index->type() == arangodb::Index::TRI_IDX_TYPE_EDGE_INDEX) { @@ -753,7 +753,7 @@ arangodb::Result PhysicalCollectionMock::read(arangodb::transaction::Methods*, arangodb::velocypack::StringRef const docKey(keySlice); if (key == docKey) { - result.setUnmanaged(doc.data(), arangodb::LocalDocumentId(i)); + result.setUnmanaged(doc.data()); return arangodb::Result(TRI_ERROR_NO_ERROR); } } @@ -784,7 +784,7 @@ bool PhysicalCollectionMock::readDocument(arangodb::transaction::Methods* trx, return false; // removed document } - result.setUnmanaged(entry.first.data(), token); + result.setUnmanaged(entry.first.data()); return true; } @@ -812,9 +812,8 @@ bool PhysicalCollectionMock::readDocumentWithCallback( arangodb::Result PhysicalCollectionMock::remove( arangodb::transaction::Methods& trx, arangodb::velocypack::Slice slice, arangodb::ManagedDocumentResult& previous, arangodb::OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& prevRev, - TRI_voc_rid_t& revisionId, arangodb::KeyLockInfo* /*keyLockInfo*/, - std::function const& callbackDuringLock) { + bool lock, arangodb::KeyLockInfo* /*keyLockInfo*/, + std::function const& callbackDuringLock) { TRI_ASSERT(callbackDuringLock == nullptr); // not implemented before(); @@ -827,14 +826,12 @@ arangodb::Result PhysicalCollectionMock::remove( continue; // removed document } - auto& doc = entry.first; + arangodb::velocypack::Builder& doc = entry.first; if (key == doc.slice().get(arangodb::StaticStrings::KeyString)) { - TRI_voc_rid_t revId = i; // always > 0 - entry.second = false; - previous.setUnmanaged(doc.data(), arangodb::LocalDocumentId(revId)); - prevRev = revId; + previous.setUnmanaged(doc.data()); + TRI_ASSERT(previous.revisionId() == TRI_ExtractRevisionId(doc.slice())); return arangodb::Result(TRI_ERROR_NO_ERROR); // assume document was removed } @@ -846,12 +843,11 @@ arangodb::Result PhysicalCollectionMock::remove( arangodb::Result PhysicalCollectionMock::replace( arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, arangodb::ManagedDocumentResult& result, - arangodb::OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool lock, TRI_voc_rid_t& prevRev, arangodb::ManagedDocumentResult& previous) { + arangodb::OperationOptions& options, + bool lock, arangodb::ManagedDocumentResult& previous) { before(); - return update(trx, newSlice, result, options, resultMarkerTick, lock, prevRev, - previous); + return update(trx, newSlice, result, options, lock, previous); } TRI_voc_rid_t PhysicalCollectionMock::revision(arangodb::transaction::Methods*) const { @@ -879,8 +875,7 @@ arangodb::Result PhysicalCollectionMock::compact() { arangodb::Result PhysicalCollectionMock::update( arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, arangodb::ManagedDocumentResult& result, arangodb::OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, TRI_voc_rid_t& prevRev, - arangodb::ManagedDocumentResult& previous) { + bool lock, arangodb::ManagedDocumentResult& previous) { auto key = newSlice.get(arangodb::StaticStrings::KeyString); if (key.isNone()) { return arangodb::Result(TRI_ERROR_ARANGO_DOCUMENT_HANDLE_BAD); @@ -898,16 +893,12 @@ arangodb::Result PhysicalCollectionMock::update( auto& doc = entry.first; if (key == doc.slice().get(arangodb::StaticStrings::KeyString)) { - TRI_voc_rid_t revId = i; // always > 0 - if (!options.mergeObjects) { entry.second = false; - previous.setUnmanaged(doc.data(), arangodb::LocalDocumentId(revId)); - prevRev = revId; + previous.setUnmanaged(doc.data()); + TRI_ASSERT(previous.revisionId() == TRI_ExtractRevisionId(doc.slice())); - TRI_voc_rid_t unused; - return insert(trx, newSlice, result, options, resultMarkerTick, lock, - unused, nullptr, nullptr); + return insert(trx, newSlice, result, options, lock, nullptr, nullptr); } arangodb::velocypack::Builder builder; @@ -928,12 +919,10 @@ arangodb::Result PhysicalCollectionMock::update( builder.close(); entry.second = false; - previous.setUnmanaged(doc.data(), arangodb::LocalDocumentId(revId)); - prevRev = revId; + previous.setUnmanaged(doc.data()); + TRI_ASSERT(previous.revisionId() == TRI_ExtractRevisionId(doc.slice())); - TRI_voc_rid_t unused; - return insert(trx, builder.slice(), result, options, resultMarkerTick, - lock, unused, nullptr, nullptr); + return insert(trx, builder.slice(), result, options, lock, nullptr, nullptr); } } diff --git a/tests/Mocks/StorageEngineMock.h b/tests/Mocks/StorageEngineMock.h index 5fe8c7362d..d547fabde3 100644 --- a/tests/Mocks/StorageEngineMock.h +++ b/tests/Mocks/StorageEngineMock.h @@ -63,10 +63,9 @@ class PhysicalCollectionMock: public arangodb::PhysicalCollection { arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, arangodb::ManagedDocumentResult& result, - arangodb::OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool lock, TRI_voc_tick_t& revisionId, - arangodb::KeyLockInfo* /*keyLockInfo*/, - std::function const& callbackDuringLock) override; + arangodb::OperationOptions& options, + bool lock, arangodb::KeyLockInfo* /*keyLockInfo*/, + std::function const& callbackDuringLock) override; virtual void invokeOnAllElements(arangodb::transaction::Methods* trx, std::function callback) override; virtual arangodb::LocalDocumentId lookupKey(arangodb::transaction::Methods*, arangodb::velocypack::Slice const&) const override; virtual size_t memory() const override; @@ -86,20 +85,16 @@ class PhysicalCollectionMock: public arangodb::PhysicalCollection { arangodb::velocypack::Slice slice, arangodb::ManagedDocumentResult& previous, arangodb::OperationOptions& options, - TRI_voc_tick_t& resultMarkerTick, bool lock, - TRI_voc_rid_t& prevRev, - TRI_voc_rid_t& revisionId, arangodb::KeyLockInfo* /*keyLockInfo*/, - std::function const& callbackDuringLock + std::function const& callbackDuringLock ) override; virtual arangodb::Result replace( arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, arangodb::ManagedDocumentResult& result, - arangodb::OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool lock, TRI_voc_rid_t& prevRev, - arangodb::ManagedDocumentResult& previous) override; + arangodb::OperationOptions& options, + bool lock, arangodb::ManagedDocumentResult& previous) override; virtual TRI_voc_rid_t revision(arangodb::transaction::Methods* trx) const override; virtual void setPath(std::string const&) override; virtual arangodb::Result truncate( @@ -111,9 +106,8 @@ class PhysicalCollectionMock: public arangodb::PhysicalCollection { arangodb::transaction::Methods* trx, arangodb::velocypack::Slice const newSlice, arangodb::ManagedDocumentResult& result, - arangodb::OperationOptions& options, TRI_voc_tick_t& resultMarkerTick, - bool lock, TRI_voc_rid_t& prevRev, - arangodb::ManagedDocumentResult& previous) override; + arangodb::OperationOptions& options, + bool lock, arangodb::ManagedDocumentResult& previous) override; virtual void load() override {} virtual void unload() override {} virtual arangodb::Result updateProperties(arangodb::velocypack::Slice const& slice, bool doSync) override;