From 2b27872e472bfc2f326eded7452d543aae2a4a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 14 Oct 2021 10:22:22 +0200 Subject: [PATCH 01/15] Took over some refactoring from a previous branches --- arangod/Cluster/ClusterTrxMethods.cpp | 8 +- arangod/Cluster/ClusterTypes.cpp | 6 ++ arangod/IResearch/IResearchAqlAnalyzer.cpp | 19 ++-- .../Methods/RocksDBReadOnlyBaseMethods.cpp | 3 - .../Methods/RocksDBReadOnlyBaseMethods.h | 2 +- .../RocksDBEngine/RocksDBTransactionState.cpp | 25 ++++- .../RocksDBEngine/RocksDBTransactionState.h | 57 +++++------- arangod/StorageEngine/TransactionState.cpp | 54 ++++++----- arangod/StorageEngine/TransactionState.h | 93 +++++++++++-------- arangod/Transaction/Hints.h | 10 +- arangod/Transaction/Manager.cpp | 38 ++++---- lib/Basics/Exceptions.h | 38 +++++++- tests/Mocks/StorageEngineMock.cpp | 4 + tests/Mocks/StorageEngineMock.h | 1 + 14 files changed, 213 insertions(+), 145 deletions(-) diff --git a/arangod/Cluster/ClusterTrxMethods.cpp b/arangod/Cluster/ClusterTrxMethods.cpp index 4743e43bb88f..e349bcbe95c9 100644 --- a/arangod/Cluster/ClusterTrxMethods.cpp +++ b/arangod/Cluster/ClusterTrxMethods.cpp @@ -271,7 +271,7 @@ Future commitAbortTransaction(arangodb::TransactionState* state, Result res; for (Try const& tryRes : responses) { network::Response const& resp = tryRes.get(); // throws exceptions upwards - Result res = ::checkTransactionResult(tidPlus, status, resp); + res = ::checkTransactionResult(tidPlus, status, resp); if (res.fail()) { break; } @@ -345,8 +345,7 @@ Future commitAbortTransaction(transaction::Methods& trx, transaction::St } // namespace -namespace arangodb { -namespace ClusterTrxMethods { +namespace arangodb::ClusterTrxMethods { using namespace arangodb::futures; bool IsServerIdLessThan::operator()(ServerID const& lhs, ServerID const& rhs) const noexcept { @@ -575,5 +574,4 @@ bool isElCheapo(TransactionState const& state) { state.hasHint(transaction::Hints::Hint::FROM_TOPLEVEL_AQL)); } -} // namespace ClusterTrxMethods -} // namespace arangodb +} // namespace arangodb::ClusterTrxMethods diff --git a/arangod/Cluster/ClusterTypes.cpp b/arangod/Cluster/ClusterTypes.cpp index aefd09338ba7..d8b1f3bd59dd 100644 --- a/arangod/Cluster/ClusterTypes.cpp +++ b/arangod/Cluster/ClusterTypes.cpp @@ -182,4 +182,10 @@ AnalyzersRevision::Ptr AnalyzersRevision::fromVelocyPack(VPackSlice const& slice buildingRevisionSlice.getNumber(), std::move(coordinatorID), rebootID)); } + +auto isShardName(std::string_view name) -> bool { + return name.size() > 1 && name[0] == 's' && + std::all_of(name.cbegin() + 1, name.cend(), static_cast(std::isdigit)); +} + } // namespace arangodb diff --git a/arangod/IResearch/IResearchAqlAnalyzer.cpp b/arangod/IResearch/IResearchAqlAnalyzer.cpp index 3f85e3058d89..3ee26c6feabc 100644 --- a/arangod/IResearch/IResearchAqlAnalyzer.cpp +++ b/arangod/IResearch/IResearchAqlAnalyzer.cpp @@ -185,36 +185,41 @@ bool normalize_slice(VPackSlice const& slice, VPackBuilder& builder) { class CalculationTransactionState final : public arangodb::TransactionState { public: explicit CalculationTransactionState(TRI_vocbase_t& vocbase) - : TransactionState(vocbase, arangodb::TransactionId(0), arangodb::transaction::Options()) { + : TransactionState(vocbase, arangodb::TransactionId(0), + arangodb::transaction::Options()) { updateStatus(arangodb::transaction::Status::RUNNING); // always running to make ASSERTS happy } - ~CalculationTransactionState() { + ~CalculationTransactionState() override { if (status() == arangodb::transaction::Status::RUNNING) { updateStatus(arangodb::transaction::Status::ABORTED); // simulate state changes to make ASSERTS happy } } /// @brief begin a transaction - arangodb::Result beginTransaction(arangodb::transaction::Hints) override { + [[nodiscard]] arangodb::Result beginTransaction(arangodb::transaction::Hints) override { return {}; } /// @brief commit a transaction - arangodb::Result commitTransaction(arangodb::transaction::Methods*) override { + [[nodiscard]] arangodb::Result commitTransaction(arangodb::transaction::Methods*) override { updateStatus(arangodb::transaction::Status::COMMITTED); // simulate state changes to make ASSERTS happy return {}; } /// @brief abort a transaction - arangodb::Result abortTransaction(arangodb::transaction::Methods*) override { + [[nodiscard]] arangodb::Result abortTransaction(arangodb::transaction::Methods*) override { updateStatus(arangodb::transaction::Status::ABORTED); // simulate state changes to make ASSERTS happy return {}; } - bool hasFailedOperations() const override { return false; } + [[nodiscard]] bool hasFailedOperations() const override { return false; } /// @brief number of commits, including intermediate commits - uint64_t numCommits() const override { return 0; } + [[nodiscard]] uint64_t numCommits() const override { return 0; } + + [[nodiscard]] TRI_voc_tick_t lastOperationTick() const noexcept override { + return 0; + } }; /// @brief Dummy transaction context which just gives dummy state diff --git a/arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.cpp b/arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.cpp index 0e8bafec8051..eaba6d9fb8be 100644 --- a/arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.cpp +++ b/arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.cpp @@ -29,9 +29,6 @@ #include using namespace arangodb; - -RocksDBReadOnlyBaseMethods::RocksDBReadOnlyBaseMethods(RocksDBTransactionState* state) - : RocksDBTransactionMethods(state) {} void RocksDBReadOnlyBaseMethods::prepareOperation(DataSourceId cid, RevisionId rid, TRI_voc_document_operation_e operationType) { diff --git a/arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.h b/arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.h index 82c6f7285c15..274bc0ac8562 100644 --- a/arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.h +++ b/arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.h @@ -33,7 +33,7 @@ namespace arangodb { class RocksDBReadOnlyBaseMethods : public RocksDBTransactionMethods { public: - explicit RocksDBReadOnlyBaseMethods(RocksDBTransactionState* state); + using RocksDBTransactionMethods::RocksDBTransactionMethods; TRI_voc_tick_t lastOperationTick() const noexcept override { return 0; } diff --git a/arangod/RocksDBEngine/RocksDBTransactionState.cpp b/arangod/RocksDBEngine/RocksDBTransactionState.cpp index 2cad520f4b76..a999bd0ea3a8 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionState.cpp +++ b/arangod/RocksDBEngine/RocksDBTransactionState.cpp @@ -280,7 +280,8 @@ Result RocksDBTransactionState::commitTransaction(transaction::Methods* activeTr cleanupTransaction(); // deletes trx ++statistics()._transactionsCommitted; } else { - abortTransaction(activeTrx); // deletes trx + // what if this fails? + std::ignore = abortTransaction(activeTrx); // deletes trx } TRI_ASSERT(!_cacheTx); @@ -450,6 +451,28 @@ rocksdb::SequenceNumber RocksDBTransactionState::beginSeq() const { return _rocksMethods->GetSequenceNumber(); } +bool RocksDBTransactionState::hasFailedOperations() const { + return (_status == transaction::Status::ABORTED) && hasOperations(); +} +RocksDBTransactionState* RocksDBTransactionState::toState(transaction::Methods* trx) { + TRI_ASSERT(trx != nullptr); + TransactionState* state = trx->state(); + TRI_ASSERT(state != nullptr); + return static_cast(state); +} +RocksDBTransactionMethods* RocksDBTransactionState::toMethods(transaction::Methods* trx) { + TRI_ASSERT(trx != nullptr); + TransactionState* state = trx->state(); + TRI_ASSERT(state != nullptr); + return static_cast(state)->rocksdbMethods(); +} +void RocksDBTransactionState::prepareForParallelReads() { _parallel = true; } +bool RocksDBTransactionState::inParallelMode() const { return _parallel; } +RocksDBTransactionMethods* RocksDBTransactionState::rocksdbMethods() { + TRI_ASSERT(_rocksMethods); + return _rocksMethods.get(); +} + #ifdef ARANGODB_ENABLE_MAINTAINER_MODE RocksDBTransactionStateGuard::RocksDBTransactionStateGuard(RocksDBTransactionState* state) noexcept : _state(state) { diff --git a/arangod/RocksDBEngine/RocksDBTransactionState.h b/arangod/RocksDBEngine/RocksDBTransactionState.h index 463fc39a1248..53384bf3a5a0 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionState.h +++ b/arangod/RocksDBEngine/RocksDBTransactionState.h @@ -66,37 +66,35 @@ class RocksDBTransactionState final : public TransactionState { public: RocksDBTransactionState(TRI_vocbase_t& vocbase, TransactionId tid, transaction::Options const& options); - ~RocksDBTransactionState(); + ~RocksDBTransactionState() override; /// @brief begin a transaction - Result beginTransaction(transaction::Hints hints) override; + [[nodiscard]] Result beginTransaction(transaction::Hints hints) override; /// @brief commit a transaction - Result commitTransaction(transaction::Methods* trx) override; + [[nodiscard]] Result commitTransaction(transaction::Methods* trx) override; /// @brief abort a transaction - Result abortTransaction(transaction::Methods* trx) override; + [[nodiscard]] Result abortTransaction(transaction::Methods* trx) override; /// @returns tick of last operation in a transaction /// @note the value is guaranteed to be valid only after /// transaction is committed - TRI_voc_tick_t lastOperationTick() const noexcept override; + [[nodiscard]] TRI_voc_tick_t lastOperationTick() const noexcept override; /// @brief number of commits, including intermediate commits - uint64_t numCommits() const override; + [[nodiscard]] uint64_t numCommits() const override; - bool hasOperations() const noexcept; + [[nodiscard]] bool hasOperations() const noexcept; - uint64_t numOperations() const noexcept; + [[nodiscard]] uint64_t numOperations() const noexcept; - bool hasFailedOperations() const override { - return (_status == transaction::Status::ABORTED) && hasOperations(); - } + [[nodiscard]] bool hasFailedOperations() const override; void beginQuery(bool isModificationQuery) override; void endQuery(bool isModificationQuery) noexcept override; - bool iteratorMustCheckBounds(ReadOwnWrites readOwnWrites) const; + [[nodiscard]] bool iteratorMustCheckBounds(ReadOwnWrites readOwnWrites) const; void prepareOperation(DataSourceId cid, RevisionId rid, TRI_voc_document_operation_e operationType); @@ -107,41 +105,28 @@ class RocksDBTransactionState final : public TransactionState { /// @brief add an operation for a transaction collection /// sets hasPerformedIntermediateCommit to true if an intermediate commit was /// performed - Result addOperation(DataSourceId collectionId, RevisionId revisionId, + [[nodiscard]] Result addOperation(DataSourceId collectionId, RevisionId revisionId, TRI_voc_document_operation_e opType, bool& hasPerformedIntermediateCommit); /// @brief return wrapper around rocksdb transaction - RocksDBTransactionMethods* rocksdbMethods() { - TRI_ASSERT(_rocksMethods); - return _rocksMethods.get(); - } + [[nodiscard]] RocksDBTransactionMethods* rocksdbMethods(); /// @brief acquire a database snapshot if we do not yet have one. /// Returns true if a snapshot was acquired, otherwise false (i.e., if we already had a snapshot) - bool ensureSnapshot(); + [[nodiscard]] bool ensureSnapshot(); - static RocksDBTransactionState* toState(transaction::Methods* trx) { - TRI_ASSERT(trx != nullptr); - TransactionState* state = trx->state(); - TRI_ASSERT(state != nullptr); - return static_cast(state); - } - - static RocksDBTransactionMethods* toMethods(transaction::Methods* trx) { - TRI_ASSERT(trx != nullptr); - TransactionState* state = trx->state(); - TRI_ASSERT(state != nullptr); - return static_cast(state)->rocksdbMethods(); - } + [[nodiscard]] static RocksDBTransactionState* toState(transaction::Methods* trx); + + [[nodiscard]] static RocksDBTransactionMethods* toMethods(transaction::Methods* trx); /// @brief make some internal preparations for accessing this state in /// parallel from multiple threads. READ-ONLY transactions - void prepareForParallelReads() { _parallel = true; } + void prepareForParallelReads(); /// @brief in parallel mode. READ-ONLY transactions - bool inParallelMode() const { return _parallel; } + [[nodiscard]] bool inParallelMode() const; - RocksDBTransactionCollection::TrackedOperations& trackedOperations(DataSourceId cid); + [[nodiscard]] RocksDBTransactionCollection::TrackedOperations& trackedOperations(DataSourceId cid); /// @brief Track documents inserted to the collection /// Used to update the revision tree for replication after commit @@ -159,9 +144,9 @@ class RocksDBTransactionState final : public TransactionState { /// Used to update the estimate after the trx committed void trackIndexRemove(DataSourceId cid, IndexId idxObjectId, uint64_t hash); - bool isOnlyExclusiveTransaction() const; + [[nodiscard]] bool isOnlyExclusiveTransaction() const; - rocksdb::SequenceNumber beginSeq() const; + [[nodiscard]] rocksdb::SequenceNumber beginSeq() const; #ifdef ARANGODB_ENABLE_MAINTAINER_MODE /// @brief only needed for RocksDBTransactionStateGuard diff --git a/arangod/StorageEngine/TransactionState.cpp b/arangod/StorageEngine/TransactionState.cpp index 32a78fa6cd9c..7d0a80d6855f 100644 --- a/arangod/StorageEngine/TransactionState.cpp +++ b/arangod/StorageEngine/TransactionState.cpp @@ -28,6 +28,7 @@ #include "Basics/DebugRaceController.h" #include "Basics/Exceptions.h" #include "Basics/StringUtils.h" +#include "Basics/overload.h" #include "Logger/LogMacros.h" #include "Logger/Logger.h" #include "Logger/LoggerStream.h" @@ -84,15 +85,18 @@ TransactionCollection* TransactionState::collection(DataSourceId cid, TRI_ASSERT(_status == transaction::Status::CREATED || _status == transaction::Status::RUNNING); - size_t unused; - TransactionCollection* trxCollection = findCollection(cid, unused); - - if (trxCollection == nullptr || !trxCollection->canAccess(accessType)) { - // not found or not accessible in the requested mode - return nullptr; - } - - return trxCollection; + auto collectionOrPos = findCollectionOrPos(cid); + + return std::visit(overload{ + [](CollectionNotFound const&) -> TransactionCollection* { + return nullptr; + }, + [&](CollectionFound const& colFound) -> TransactionCollection* { + auto* const col = colFound.collection; + return col->canAccess(accessType) ? col : nullptr; + }, + }, + collectionOrPos); } /// @brief return the collection from a transaction @@ -114,7 +118,7 @@ TransactionCollection* TransactionState::collection(std::string const& name, return (*it); } -TransactionState::Cookie* TransactionState::cookie(void const* key) noexcept { +TransactionState::Cookie* TransactionState::cookie(void const* key) const noexcept { auto itr = _cookies.find(key); return itr == _cookies.end() ? nullptr : itr->second.get(); @@ -194,10 +198,9 @@ Result TransactionState::addCollectionInternal(DataSourceId cid, std::string con Result res; // check if we already got this collection in the _collections vector - size_t position = 0; - TransactionCollection* trxColl = findCollection(cid, position); - - if (trxColl != nullptr) { + auto colOrPos = findCollectionOrPos(cid); + if (std::holds_alternative(colOrPos)) { + auto* const trxColl = std::get(colOrPos).collection; LOG_TRX("ad6d0", TRACE, this) << "updating collection usage " << cid << ": '" << cname << "'"; @@ -212,6 +215,9 @@ Result TransactionState::addCollectionInternal(DataSourceId cid, std::string con // collection is already contained in vector return res.reset(trxColl->updateUsage(accessType)); } + TRI_ASSERT(std::holds_alternative(colOrPos)); + auto const position = std::get(colOrPos).lowerBound; + // collection not found. @@ -242,11 +248,10 @@ Result TransactionState::addCollectionInternal(DataSourceId cid, std::string con } // collection was not contained. now create and insert it - TRI_ASSERT(trxColl == nullptr); StorageEngine& engine = vocbase().server().getFeature().engine(); - trxColl = engine.createTransactionCollection(*this, cid, accessType).release(); + auto* const trxColl = engine.createTransactionCollection(*this, cid, accessType).release(); TRI_ASSERT(trxColl != nullptr); @@ -302,21 +307,22 @@ TransactionCollection* TransactionState::findCollection(DataSourceId cid) const /// The idea is if a collection is found it will be returned. /// In this case the position is not used. /// In case the collection is not found. It will return a -/// nullptr and the position will be set. The position +/// lower bound of its position. The position /// defines where the collection should be inserted, /// so whenever we want to insert the collection we /// have to use this position for insert. -TransactionCollection* TransactionState::findCollection(DataSourceId cid, - size_t& position) const { +auto TransactionState::findCollectionOrPos(DataSourceId cid) const + -> std::variant { size_t const n = _collections.size(); size_t i; + // TODO We could do a binary search here. for (i = 0; i < n; ++i) { - auto trxCollection = _collections[i]; + auto* trxCollection = _collections[i]; if (cid == trxCollection->id()) { // found - return trxCollection; + return CollectionFound{trxCollection}; } if (cid < trxCollection->id()) { @@ -326,10 +332,8 @@ TransactionCollection* TransactionState::findCollection(DataSourceId cid, // next } - // update the insert position if required - position = i; - - return nullptr; + // return the insert position if required + return CollectionNotFound{i}; } void TransactionState::setExclusiveAccessType() { diff --git a/arangod/StorageEngine/TransactionState.h b/arangod/StorageEngine/TransactionState.h index bdf94ce3951b..545897d636fc 100644 --- a/arangod/StorageEngine/TransactionState.h +++ b/arangod/StorageEngine/TransactionState.h @@ -73,7 +73,7 @@ class TransactionState { virtual ~Cookie() = default; }; - static bool ServerIdLessThan(ServerID const& lhs, ServerID const& rhs) { + [[nodiscard]] static bool ServerIdLessThan(ServerID const& lhs, ServerID const& rhs) { return lhs < rhs; } @@ -88,51 +88,59 @@ class TransactionState { virtual ~TransactionState(); /// @return a cookie associated with the specified key, nullptr if none - Cookie* cookie(void const* key) noexcept; + [[nodiscard]] Cookie* cookie(void const* key) const noexcept; /// @brief associate the specified cookie with the specified key /// @return the previously associated cookie, if any Cookie::ptr cookie(void const* key, Cookie::ptr&& cookie); - bool isRunningInCluster() const { + [[nodiscard]] bool isRunningInCluster() const { return ServerState::isRunningInCluster(_serverRole); } - bool isDBServer() const { return ServerState::isDBServer(_serverRole); } - bool isCoordinator() const { return ServerState::isCoordinator(_serverRole); } - ServerState::RoleEnum serverRole() const { return _serverRole; } - - inline transaction::Options& options() { return _options; } - inline transaction::Options const& options() const { return _options; } - inline TRI_vocbase_t& vocbase() const { return _vocbase; } - inline TransactionId id() const { return _id; } - inline transaction::Status status() const { return _status; } - inline bool isRunning() const { + [[nodiscard]] bool isDBServer() const { + return ServerState::isDBServer(_serverRole); + } + [[nodiscard]] bool isCoordinator() const { + return ServerState::isCoordinator(_serverRole); + } + [[nodiscard]] ServerState::RoleEnum serverRole() const { return _serverRole; } + + [[nodiscard]] transaction::Options& options() { return _options; } + [[nodiscard]] transaction::Options const& options() const { + return _options; + } + [[nodiscard]] TRI_vocbase_t& vocbase() const { return _vocbase; } + [[nodiscard]] TransactionId id() const { return _id; } + [[nodiscard]] transaction::Status status() const noexcept { return _status; } + [[nodiscard]] bool isRunning() const { return _status == transaction::Status::RUNNING; } void setRegistered() noexcept { _registeredTransaction = true; } - bool wasRegistered() const noexcept { return _registeredTransaction; } + [[nodiscard]] bool wasRegistered() const noexcept { + return _registeredTransaction; + } /// @brief returns the name of the actor the transaction runs on: /// - leader /// - follower /// - coordinator /// - single - char const* actorName() const noexcept; + [[nodiscard]] char const* actorName() const noexcept; /// @brief return a reference to the global transaction statistics/counters TransactionStatistics& statistics() noexcept; - double lockTimeout() const { return _options.lockTimeout; } + [[nodiscard]] double lockTimeout() const { return _options.lockTimeout; } void lockTimeout(double value) { if (value > 0.0) { _options.lockTimeout = value; } } - bool waitForSync() const { return _options.waitForSync; } + [[nodiscard]] bool waitForSync() const { return _options.waitForSync; } void waitForSync(bool value) { _options.waitForSync = value; } - bool allowImplicitCollectionsForRead() const { + [[nodiscard]] bool allowImplicitCollectionsForRead() const { return _options.allowImplicitCollectionsForRead; } void allowImplicitCollectionsForRead(bool value) { @@ -140,17 +148,19 @@ class TransactionState { } /// @brief return the collection from a transaction - TransactionCollection* collection(DataSourceId cid, AccessMode::Type accessType) const; + [[nodiscard]] TransactionCollection* collection(DataSourceId cid, + AccessMode::Type accessType) const; /// @brief return the collection from a transaction - TransactionCollection* collection(std::string const& name, AccessMode::Type accessType) const; + [[nodiscard]] TransactionCollection* collection(std::string const& name, + AccessMode::Type accessType) const; /// @brief add a collection to a transaction - Result addCollection(DataSourceId cid, std::string const& cname, - AccessMode::Type accessType, bool lockUsage); + [[nodiscard]] Result addCollection(DataSourceId cid, std::string const& cname, + AccessMode::Type accessType, bool lockUsage); /// @brief use all participating collections of a transaction - Result useCollections(); + [[nodiscard]] Result useCollections(); /// @brief run a callback on all collections of the transaction template @@ -164,10 +174,10 @@ class TransactionState { } /// @brief return the number of collections in the transaction - size_t numCollections() const { return _collections.size(); } + [[nodiscard]] size_t numCollections() const { return _collections.size(); } /// @brief whether or not a transaction consists of a single operation - bool isSingleOperation() const { + [[nodiscard]] bool isSingleOperation() const { return hasHint(transaction::Hints::Hint::SINGLE_OPERATION); } @@ -175,7 +185,9 @@ class TransactionState { void updateStatus(transaction::Status status) noexcept; /// @brief whether or not a specific hint is set for the transaction - bool hasHint(transaction::Hints::Hint hint) const { return _hints.has(hint); } + [[nodiscard]] bool hasHint(transaction::Hints::Hint hint) const { + return _hints.has(hint); + } /// @brief begin a transaction virtual arangodb::Result beginTransaction(transaction::Hints hints) = 0; @@ -197,18 +209,18 @@ class TransactionState { virtual void beginQuery(bool /*isModificationQuery*/) {} virtual void endQuery(bool /*isModificationQuery*/) noexcept {} - TransactionCollection* findCollection(DataSourceId cid) const; + [[nodiscard]] TransactionCollection* findCollection(DataSourceId cid) const; /// @brief make a exclusive transaction, only valid before begin void setExclusiveAccessType(); /// @brief whether or not a transaction is read-only - bool isReadOnlyTransaction() const { - return (_type == AccessMode::Type::READ); + [[nodiscard]] bool isReadOnlyTransaction() const noexcept { + return _type == AccessMode::Type::READ; } /// @brief whether or not a transaction is a follower transaction - bool isFollowerTransaction() const { + [[nodiscard]] bool isFollowerTransaction() const { return hasHint(transaction::Hints::Hint::IS_FOLLOWER_TRX); } @@ -216,11 +228,11 @@ class TransactionState { bool isOnlyExclusiveTransaction() const; /// @brief servers already contacted - ::arangodb::containers::HashSet const& knownServers() const { + [[nodiscard]] ::arangodb::containers::HashSet const& knownServers() const { return _knownServers; } - bool knowsServer(std::string const& uuid) const { + [[nodiscard]] bool knowsServer(std::string const& uuid) const { return _knownServers.find(uuid) != _knownServers.end(); } @@ -235,18 +247,18 @@ class TransactionState { /// @returns tick of last operation in a transaction /// @note the value is guaranteed to be valid only after /// transaction is committed - virtual TRI_voc_tick_t lastOperationTick() const noexcept { return 0; } + [[nodiscard]] virtual TRI_voc_tick_t lastOperationTick() const noexcept = 0; void acceptAnalyzersRevision(QueryAnalyzerRevisions const& analyzersRevsion) noexcept; - const QueryAnalyzerRevisions& analyzersRevision() const noexcept { + [[nodiscard]] QueryAnalyzerRevisions const& analyzersRevision() const noexcept { return _analyzersRevision; } #ifdef USE_ENTERPRISE void addInaccessibleCollection(DataSourceId cid, std::string const& cname); - bool isInaccessibleCollection(DataSourceId cid); - bool isInaccessibleCollection(std::string const& cname); + [[nodiscard]] bool isInaccessibleCollection(DataSourceId cid); + [[nodiscard]] bool isInaccessibleCollection(std::string const& cname); #endif /// @brief roll a new transaction ID on the coordintor. Use this method @@ -257,7 +269,14 @@ class TransactionState { protected: /// @brief find a collection in the transaction's list of collections - TransactionCollection* findCollection(DataSourceId cid, size_t& position) const; + struct CollectionNotFound { + std::size_t lowerBound; + }; + struct CollectionFound { + TransactionCollection* collection; + }; + [[nodiscard]] auto findCollectionOrPos(DataSourceId cid) const + -> std::variant; /// @brief clear the query cache for all collections that were modified by /// the transaction diff --git a/arangod/Transaction/Hints.h b/arangod/Transaction/Hints.h index 5d4320cb9a6c..0a09d4af5ad9 100644 --- a/arangod/Transaction/Hints.h +++ b/arangod/Transaction/Hints.h @@ -23,14 +23,13 @@ #pragma once -#include "Basics/Common.h" +#include -namespace arangodb { -namespace transaction { +namespace arangodb::transaction { class Hints { public: - typedef uint32_t ValueType; + typedef std::uint32_t ValueType; /// @brief individual hint flags that can be used for transactions enum class Hint : ValueType { @@ -76,6 +75,5 @@ class Hints { ValueType _value; }; -} // namespace transaction -} // namespace arangodb +} // namespace arangodb::transaction diff --git a/arangod/Transaction/Manager.cpp b/arangod/Transaction/Manager.cpp index 4c863e7f1c87..e4e38ff2a2ea 100644 --- a/arangod/Transaction/Manager.cpp +++ b/arangod/Transaction/Manager.cpp @@ -359,6 +359,7 @@ ResultT Manager::createManagedTrx(TRI_vocbase_t& vocbase, VPackSl Result Manager::ensureManagedTrx(TRI_vocbase_t& vocbase, TransactionId tid, VPackSlice trxOpts, bool isFollowerTransaction) { + TRI_ASSERT(tid.isFollowerTransactionId() == isFollowerTransaction); transaction::Options options; std::vector reads, writes, exclusives; @@ -367,10 +368,6 @@ Result Manager::ensureManagedTrx(TRI_vocbase_t& vocbase, TransactionId tid, return res; } - if (isFollowerTransaction) { - options.isFollowerTransaction = true; - } - return ensureManagedTrx(vocbase, tid, reads, writes, exclusives, std::move(options)); } @@ -562,20 +559,23 @@ ResultT Manager::createManagedTrx( if (res.fail()) { return res; } - std::shared_ptr state; ServerState::RoleEnum role = ServerState::instance()->getRole(); TRI_ASSERT(ServerState::isSingleServerOrCoordinator(role)); TransactionId tid = ServerState::isSingleServer(role) ? TransactionId::createSingleServer() : TransactionId::createCoordinator(); - try { - // now start our own transaction + + auto maybeState = basics::catchToResultT([&] { StorageEngine& engine = vocbase.server().getFeature().engine(); - state = engine.createTransactionState(vocbase, tid, options); - } catch (basics::Exception const& e) { - return res.reset(e.code(), e.message()); + // now start our own transaction + return engine.createTransactionState(vocbase, tid, options); + }); + if (!maybeState.ok()) { + return std::move(maybeState).result(); } + auto& state = maybeState.get(); + TRI_ASSERT(state != nullptr); TRI_ASSERT(state->id() == tid); @@ -627,9 +627,7 @@ Result Manager::ensureManagedTrx(TRI_vocbase_t& vocbase, TransactionId tid, return res.reset(TRI_ERROR_SHUTTING_DOWN); } - if (tid.isFollowerTransactionId()) { - options.isFollowerTransaction = true; - } + options.isFollowerTransaction = tid.isFollowerTransactionId(); LOG_TOPIC("7bd2d", DEBUG, Logger::TRANSACTIONS) << "managed trx creating: " << tid.id(); @@ -661,14 +659,16 @@ Result Manager::ensureManagedTrx(TRI_vocbase_t& vocbase, TransactionId tid, return res; } - std::shared_ptr state; - try { - // now start our own transaction + auto maybeState = basics::catchToResultT([&] { StorageEngine& engine = vocbase.server().getFeature().engine(); - state = engine.createTransactionState(vocbase, tid, options); - } catch (basics::Exception const& e) { - return res.reset(e.code(), e.message()); + // now start our own transaction + return engine.createTransactionState(vocbase, tid, options); + }); + if (!maybeState.ok()) { + return std::move(maybeState).result(); } + auto& state = maybeState.get(); + TRI_ASSERT(state != nullptr); TRI_ASSERT(state->id() == tid); diff --git a/lib/Basics/Exceptions.h b/lib/Basics/Exceptions.h index c10f7d29435f..e8c64703a397 100644 --- a/lib/Basics/Exceptions.h +++ b/lib/Basics/Exceptions.h @@ -30,6 +30,7 @@ #include #include "Basics/Result.h" +#include "Basics/ResultT.h" #include "Basics/SourceLocation.h" #include "Basics/StringUtils.h" #include "Basics/error.h" @@ -127,7 +128,7 @@ namespace helper { } // namespace helper template -Result catchToResult(F&& fn, ErrorCode defaultError = TRI_ERROR_INTERNAL) noexcept { +[[nodiscard]] auto catchToResult(F&& fn) noexcept -> Result { Result result{TRI_ERROR_NO_ERROR}; // The outer try/catch catches possible exceptions thrown by result.reset(), // due to allocation failure. If we don't have enough memory to allocate an @@ -141,9 +142,36 @@ Result catchToResult(F&& fn, ErrorCode defaultError = TRI_ERROR_INTERNAL) noexce } catch (std::bad_alloc const&) { result.reset(TRI_ERROR_OUT_OF_MEMORY); } catch (std::exception const& e) { - result.reset(defaultError, e.what()); + result.reset(TRI_ERROR_INTERNAL, e.what()); } catch (...) { - result.reset(defaultError); + result.reset(TRI_ERROR_INTERNAL); + } + } catch (std::exception const& e) { + helper::dieWithLogMessage(e.what()); + } catch (...) { + helper::dieWithLogMessage(nullptr); + } + return result; +} + +template > +[[nodiscard]] auto catchToResultT(F&& fn) noexcept -> ResultT { + Result result{TRI_ERROR_NO_ERROR}; + // The outer try/catch catches possible exceptions thrown by result.reset(), + // due to allocation failure. If we don't have enough memory to allocate an + // error, let's just give up. + try { + try { + result = std::forward(fn)(); + // TODO check whether there are other specific exceptions we should catch + } catch (arangodb::basics::Exception const& e) { + result.reset(e.code(), e.message()); + } catch (std::bad_alloc const&) { + result.reset(TRI_ERROR_OUT_OF_MEMORY); + } catch (std::exception const& e) { + result.reset(TRI_ERROR_INTERNAL, e.what()); + } catch (...) { + result.reset(TRI_ERROR_INTERNAL); } } catch (std::exception const& e) { helper::dieWithLogMessage(e.what()); @@ -154,12 +182,12 @@ Result catchToResult(F&& fn, ErrorCode defaultError = TRI_ERROR_INTERNAL) noexce } template -Result catchVoidToResult(F&& fn, ErrorCode defaultError = TRI_ERROR_INTERNAL) noexcept { +[[nodiscard]] auto catchVoidToResult(F&& fn) noexcept -> Result { auto wrapped = [&fn]() -> Result { std::forward(fn)(); return Result{TRI_ERROR_NO_ERROR}; }; - return catchToResult(wrapped, defaultError); + return catchToResult(wrapped); } namespace helper { diff --git a/tests/Mocks/StorageEngineMock.cpp b/tests/Mocks/StorageEngineMock.cpp index 146a5b19d912..7b5654d6ed73 100644 --- a/tests/Mocks/StorageEngineMock.cpp +++ b/tests/Mocks/StorageEngineMock.cpp @@ -1977,6 +1977,10 @@ bool TransactionStateMock::hasFailedOperations() const { return false; // assume no failed operations } +TRI_voc_tick_t TransactionStateMock::lastOperationTick() const noexcept { + return 0; +} + // ----------------------------------------------------------------------------- // --SECTION-- END-OF-FILE // ----------------------------------------------------------------------------- diff --git a/tests/Mocks/StorageEngineMock.h b/tests/Mocks/StorageEngineMock.h index 8ec1ace54623..2213906a8e7b 100644 --- a/tests/Mocks/StorageEngineMock.h +++ b/tests/Mocks/StorageEngineMock.h @@ -179,6 +179,7 @@ class TransactionStateMock : public arangodb::TransactionState { virtual arangodb::Result commitTransaction(arangodb::transaction::Methods* trx) override; virtual uint64_t numCommits() const override; virtual bool hasFailedOperations() const override; + TRI_voc_tick_t lastOperationTick() const noexcept override; }; class StorageEngineMock : public arangodb::StorageEngine { From 4fdfde9ec7d5e325c8f26798f73719b74969feae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 14 Oct 2021 10:29:14 +0200 Subject: [PATCH 02/15] Took over some more changes --- .../ClusterEngine/ClusterTransactionState.cpp | 4 +++ .../ClusterEngine/ClusterTransactionState.h | 25 +++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/arangod/ClusterEngine/ClusterTransactionState.cpp b/arangod/ClusterEngine/ClusterTransactionState.cpp index 7ad4cb87cd99..8c28d4615469 100644 --- a/arangod/ClusterEngine/ClusterTransactionState.cpp +++ b/arangod/ClusterEngine/ClusterTransactionState.cpp @@ -153,3 +153,7 @@ uint64_t ClusterTransactionState::numCommits() const { // return 1 for a committed transaction and 0 otherwise return _status == transaction::Status::COMMITTED ? 1 : 0; } + +TRI_voc_tick_t ClusterTransactionState::lastOperationTick() const noexcept { + return 0; +} diff --git a/arangod/ClusterEngine/ClusterTransactionState.h b/arangod/ClusterEngine/ClusterTransactionState.h index bebbb392a669..7a9f4d998e74 100644 --- a/arangod/ClusterEngine/ClusterTransactionState.h +++ b/arangod/ClusterEngine/ClusterTransactionState.h @@ -24,30 +24,39 @@ #pragma once #include "StorageEngine/TransactionState.h" +#include "VocBase/Identifiers/TransactionId.h" + +struct TRI_vocbase_t; namespace arangodb { +class Result; + +namespace transaction { +struct Options; +} /// @brief transaction type class ClusterTransactionState final : public TransactionState { public: ClusterTransactionState(TRI_vocbase_t& vocbase, TransactionId tid, transaction::Options const& options); - ~ClusterTransactionState() = default; + ~ClusterTransactionState() override = default; /// @brief begin a transaction - Result beginTransaction(transaction::Hints hints) override; + [[nodiscard]] Result beginTransaction(transaction::Hints hints) override; /// @brief commit a transaction - Result commitTransaction(transaction::Methods* trx) override; + [[nodiscard]] Result commitTransaction(transaction::Methods* trx) override; /// @brief abort a transaction - Result abortTransaction(transaction::Methods* trx) override; - + [[nodiscard]] Result abortTransaction(transaction::Methods* trx) override; + /// @brief return number of commits, including intermediate commits - uint64_t numCommits() const override; + [[nodiscard]] uint64_t numCommits() const override; - bool hasFailedOperations() const override { return false; } + [[nodiscard]] bool hasFailedOperations() const override { return false; } + + [[nodiscard]] TRI_voc_tick_t lastOperationTick() const noexcept override; }; } // namespace arangodb - From a0933e1ce299f12b70cca6b1857ac8b4e0faf3bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 14 Oct 2021 10:30:49 +0200 Subject: [PATCH 03/15] Fixes --- arangod/RocksDBEngine/RocksDBTransactionState.cpp | 12 ------------ arangod/RocksDBEngine/RocksDBTransactionState.h | 2 -- arangod/StorageEngine/TransactionState.h | 1 + 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/arangod/RocksDBEngine/RocksDBTransactionState.cpp b/arangod/RocksDBEngine/RocksDBTransactionState.cpp index a999bd0ea3a8..dc0511c31dda 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionState.cpp +++ b/arangod/RocksDBEngine/RocksDBTransactionState.cpp @@ -435,18 +435,6 @@ void RocksDBTransactionState::trackIndexRemove(DataSourceId cid, IndexId idxId, } } -bool RocksDBTransactionState::isOnlyExclusiveTransaction() const { - if (!AccessMode::isWriteOrExclusive(_type)) { - return false; - } - for (TransactionCollection* coll : _collections) { - if (AccessMode::isWrite(coll->accessType())) { - return false; - } - } - return true; -} - rocksdb::SequenceNumber RocksDBTransactionState::beginSeq() const { return _rocksMethods->GetSequenceNumber(); } diff --git a/arangod/RocksDBEngine/RocksDBTransactionState.h b/arangod/RocksDBEngine/RocksDBTransactionState.h index 53384bf3a5a0..0242f20bbf65 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionState.h +++ b/arangod/RocksDBEngine/RocksDBTransactionState.h @@ -144,8 +144,6 @@ class RocksDBTransactionState final : public TransactionState { /// Used to update the estimate after the trx committed void trackIndexRemove(DataSourceId cid, IndexId idxObjectId, uint64_t hash); - [[nodiscard]] bool isOnlyExclusiveTransaction() const; - [[nodiscard]] rocksdb::SequenceNumber beginSeq() const; #ifdef ARANGODB_ENABLE_MAINTAINER_MODE diff --git a/arangod/StorageEngine/TransactionState.h b/arangod/StorageEngine/TransactionState.h index 545897d636fc..fb11403c6e89 100644 --- a/arangod/StorageEngine/TransactionState.h +++ b/arangod/StorageEngine/TransactionState.h @@ -38,6 +38,7 @@ #include "VocBase/voc-types.h" #include +#include #ifdef ARANGODB_ENABLE_MAINTAINER_MODE From 4f0766c76412e784758a39d11a538f9d7e361f8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 14 Oct 2021 10:48:07 +0200 Subject: [PATCH 04/15] Fixed catchToResultT --- lib/Basics/Exceptions.h | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/Basics/Exceptions.h b/lib/Basics/Exceptions.h index e8c64703a397..411eb79cde03 100644 --- a/lib/Basics/Exceptions.h +++ b/lib/Basics/Exceptions.h @@ -129,56 +129,52 @@ namespace helper { template [[nodiscard]] auto catchToResult(F&& fn) noexcept -> Result { - Result result{TRI_ERROR_NO_ERROR}; // The outer try/catch catches possible exceptions thrown by result.reset(), // due to allocation failure. If we don't have enough memory to allocate an // error, let's just give up. try { try { - result = std::forward(fn)(); + return std::forward(fn)(); // TODO check whether there are other specific exceptions we should catch } catch (arangodb::basics::Exception const& e) { - result.reset(e.code(), e.message()); + return Result(e.code(), e.message()); } catch (std::bad_alloc const&) { - result.reset(TRI_ERROR_OUT_OF_MEMORY); + return Result(TRI_ERROR_OUT_OF_MEMORY); } catch (std::exception const& e) { - result.reset(TRI_ERROR_INTERNAL, e.what()); + return Result(TRI_ERROR_INTERNAL, e.what()); } catch (...) { - result.reset(TRI_ERROR_INTERNAL); + return Result(TRI_ERROR_INTERNAL); } } catch (std::exception const& e) { helper::dieWithLogMessage(e.what()); } catch (...) { helper::dieWithLogMessage(nullptr); } - return result; } template > [[nodiscard]] auto catchToResultT(F&& fn) noexcept -> ResultT { - Result result{TRI_ERROR_NO_ERROR}; // The outer try/catch catches possible exceptions thrown by result.reset(), // due to allocation failure. If we don't have enough memory to allocate an // error, let's just give up. try { try { - result = std::forward(fn)(); + return std::forward(fn)(); // TODO check whether there are other specific exceptions we should catch } catch (arangodb::basics::Exception const& e) { - result.reset(e.code(), e.message()); + return ResultT::error(e.code(), e.message()); } catch (std::bad_alloc const&) { - result.reset(TRI_ERROR_OUT_OF_MEMORY); + return ResultT::error(TRI_ERROR_OUT_OF_MEMORY); } catch (std::exception const& e) { - result.reset(TRI_ERROR_INTERNAL, e.what()); + return ResultT::error(TRI_ERROR_INTERNAL, e.what()); } catch (...) { - result.reset(TRI_ERROR_INTERNAL); + return ResultT::error(TRI_ERROR_INTERNAL); } } catch (std::exception const& e) { helper::dieWithLogMessage(e.what()); } catch (...) { helper::dieWithLogMessage(nullptr); } - return result; } template From c56cca990c171f6c45c5e5dd70404e90b533b460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 14 Oct 2021 11:30:11 +0200 Subject: [PATCH 05/15] Re-added missing function --- arangod/RocksDBEngine/RocksDBTransactionState.cpp | 9 +++++++++ arangod/RocksDBEngine/RocksDBTransactionState.h | 3 +++ arangod/StorageEngine/TransactionCollection.h | 2 +- arangod/StorageEngine/TransactionState.h | 3 --- arangod/VocBase/AccessMode.h | 12 ++++++------ 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/arangod/RocksDBEngine/RocksDBTransactionState.cpp b/arangod/RocksDBEngine/RocksDBTransactionState.cpp index dc0511c31dda..2f8788f807da 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionState.cpp +++ b/arangod/RocksDBEngine/RocksDBTransactionState.cpp @@ -435,6 +435,15 @@ void RocksDBTransactionState::trackIndexRemove(DataSourceId cid, IndexId idxId, } } +bool RocksDBTransactionState::isOnlyExclusiveTransaction() const noexcept { + if (!AccessMode::isWriteOrExclusive(_type)) { + return false; + } + return std::none_of(_collections.cbegin(), _collections.cend(), [](auto* coll) { + return AccessMode::isWrite(coll->accessType()); + }); +} + rocksdb::SequenceNumber RocksDBTransactionState::beginSeq() const { return _rocksMethods->GetSequenceNumber(); } diff --git a/arangod/RocksDBEngine/RocksDBTransactionState.h b/arangod/RocksDBEngine/RocksDBTransactionState.h index 0242f20bbf65..5036c856e55e 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionState.h +++ b/arangod/RocksDBEngine/RocksDBTransactionState.h @@ -144,6 +144,9 @@ class RocksDBTransactionState final : public TransactionState { /// Used to update the estimate after the trx committed void trackIndexRemove(DataSourceId cid, IndexId idxObjectId, uint64_t hash); + /// @brief whether or not a transaction only has exclusive or read accesses + bool isOnlyExclusiveTransaction() const noexcept; + [[nodiscard]] rocksdb::SequenceNumber beginSeq() const; #ifdef ARANGODB_ENABLE_MAINTAINER_MODE diff --git a/arangod/StorageEngine/TransactionCollection.h b/arangod/StorageEngine/TransactionCollection.h index e1e0b920fb57..08d0c99f23ac 100644 --- a/arangod/StorageEngine/TransactionCollection.h +++ b/arangod/StorageEngine/TransactionCollection.h @@ -60,7 +60,7 @@ class TransactionCollection { std::string const& collectionName() const; - AccessMode::Type accessType() const { return _accessType; } + AccessMode::Type accessType() const noexcept { return _accessType; } Result updateUsage(AccessMode::Type accessType); diff --git a/arangod/StorageEngine/TransactionState.h b/arangod/StorageEngine/TransactionState.h index fb11403c6e89..25201e79dfb3 100644 --- a/arangod/StorageEngine/TransactionState.h +++ b/arangod/StorageEngine/TransactionState.h @@ -225,9 +225,6 @@ class TransactionState { return hasHint(transaction::Hints::Hint::IS_FOLLOWER_TRX); } - /// @brief whether or not a transaction only has exculsive or read accesses - bool isOnlyExclusiveTransaction() const; - /// @brief servers already contacted [[nodiscard]] ::arangodb::containers::HashSet const& knownServers() const { return _knownServers; diff --git a/arangod/VocBase/AccessMode.h b/arangod/VocBase/AccessMode.h index cc34cba00935..ee77a6ea7fcf 100644 --- a/arangod/VocBase/AccessMode.h +++ b/arangod/VocBase/AccessMode.h @@ -40,16 +40,16 @@ struct AccessMode { AccessMode::Type::WRITE < AccessMode::Type::EXCLUSIVE, "AccessMode::Type total order fail"); - static bool isNone(Type type) { return (type == Type::NONE); } + static bool isNone(Type type) noexcept { return type == Type::NONE; } - static bool isRead(Type type) { return (type == Type::READ); } + static bool isRead(Type type) noexcept { return type == Type::READ; } - static bool isWrite(Type type) { return (type == Type::WRITE); } + static bool isWrite(Type type) noexcept { return type == Type::WRITE; } - static bool isExclusive(Type type) { return (type == Type::EXCLUSIVE); } + static bool isExclusive(Type type) noexcept { return type == Type::EXCLUSIVE; } - static bool isWriteOrExclusive(Type type) { - return (isWrite(type) || isExclusive(type)); + static bool isWriteOrExclusive(Type type) noexcept { + return isWrite(type) || isExclusive(type); } /// @brief checks if the type of the two modes is different From 75462d90d551c99dca7f6a9dc39f31b8439c8b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 14 Oct 2021 11:32:11 +0200 Subject: [PATCH 06/15] Moved default initializations --- arangod/StorageEngine/TransactionState.cpp | 7 +------ arangod/StorageEngine/TransactionState.h | 10 +++++----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/arangod/StorageEngine/TransactionState.cpp b/arangod/StorageEngine/TransactionState.cpp index 7d0a80d6855f..a7c49427183d 100644 --- a/arangod/StorageEngine/TransactionState.cpp +++ b/arangod/StorageEngine/TransactionState.cpp @@ -52,15 +52,10 @@ using namespace arangodb; TransactionState::TransactionState(TRI_vocbase_t& vocbase, TransactionId tid, transaction::Options const& options) : _vocbase(vocbase), - _type(AccessMode::Type::READ), - _status(transaction::Status::CREATED), - _arena(), _collections{_arena}, // assign arena to vector - _hints(), _serverRole(ServerState::instance()->getRole()), _options(options), - _id(tid), - _registeredTransaction(false) { + _id(tid) { // patch intermediateCommitCount for testing #ifdef ARANGODB_ENABLE_FAILURE_TESTS diff --git a/arangod/StorageEngine/TransactionState.h b/arangod/StorageEngine/TransactionState.h index 25201e79dfb3..ad74afe2c9f9 100644 --- a/arangod/StorageEngine/TransactionState.h +++ b/arangod/StorageEngine/TransactionState.h @@ -299,15 +299,15 @@ class TransactionState { TRI_vocbase_t& _vocbase; /// @brief vocbase for this transaction /// @brief access type (read|write) - AccessMode::Type _type; + AccessMode::Type _type = AccessMode::Type::READ; /// @brief current status - transaction::Status _status; + transaction::Status _status = transaction::Status::CREATED; using ListType = arangodb::containers::SmallVector; - ListType::allocator_type::arena_type _arena; // memory for collections + ListType::allocator_type::arena_type _arena{}; // memory for collections ListType _collections; // list of participating collections - transaction::Hints _hints; // hints; set on _nestingLevel == 0 + transaction::Hints _hints{}; // hints; set on _nestingLevel == 0 ServerState::RoleEnum const _serverRole; /// role of the server @@ -323,7 +323,7 @@ class TransactionState { ::arangodb::containers::HashSet _knownServers; QueryAnalyzerRevisions _analyzersRevision; - bool _registeredTransaction; + bool _registeredTransaction = false; }; } // namespace arangodb From 391c7bf5d717a2567d35fdb8819a48ea5633b62c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 21 Oct 2021 10:36:42 +0200 Subject: [PATCH 07/15] Fixed C++ tests --- arangod/Transaction/Manager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arangod/Transaction/Manager.cpp b/arangod/Transaction/Manager.cpp index e4e38ff2a2ea..623ec46b392a 100644 --- a/arangod/Transaction/Manager.cpp +++ b/arangod/Transaction/Manager.cpp @@ -359,7 +359,8 @@ ResultT Manager::createManagedTrx(TRI_vocbase_t& vocbase, VPackSl Result Manager::ensureManagedTrx(TRI_vocbase_t& vocbase, TransactionId tid, VPackSlice trxOpts, bool isFollowerTransaction) { - TRI_ASSERT(tid.isFollowerTransactionId() == isFollowerTransaction); + TRI_ASSERT(ServerState::instance()->isSingleServer() || + tid.isFollowerTransactionId() == isFollowerTransaction); transaction::Options options; std::vector reads, writes, exclusives; From a77d0fa82d285f4ac99c65d3aad70ee975db385b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 21 Oct 2021 10:38:03 +0200 Subject: [PATCH 08/15] Improved the assertion on single server a little --- arangod/Transaction/Manager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/Transaction/Manager.cpp b/arangod/Transaction/Manager.cpp index 623ec46b392a..7bf5bcd4194a 100644 --- a/arangod/Transaction/Manager.cpp +++ b/arangod/Transaction/Manager.cpp @@ -359,7 +359,7 @@ ResultT Manager::createManagedTrx(TRI_vocbase_t& vocbase, VPackSl Result Manager::ensureManagedTrx(TRI_vocbase_t& vocbase, TransactionId tid, VPackSlice trxOpts, bool isFollowerTransaction) { - TRI_ASSERT(ServerState::instance()->isSingleServer() || + TRI_ASSERT(ServerState::instance()->isSingleServer() && !isFollowerTransaction || tid.isFollowerTransactionId() == isFollowerTransaction); transaction::Options options; std::vector reads, writes, exclusives; From 5853ec091cb2d0422545ee2421ce57a58a670f86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 21 Oct 2021 13:13:16 +0200 Subject: [PATCH 09/15] Suppressed warning --- arangod/Transaction/Manager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/Transaction/Manager.cpp b/arangod/Transaction/Manager.cpp index 7bf5bcd4194a..7df0e803d7ec 100644 --- a/arangod/Transaction/Manager.cpp +++ b/arangod/Transaction/Manager.cpp @@ -359,7 +359,7 @@ ResultT Manager::createManagedTrx(TRI_vocbase_t& vocbase, VPackSl Result Manager::ensureManagedTrx(TRI_vocbase_t& vocbase, TransactionId tid, VPackSlice trxOpts, bool isFollowerTransaction) { - TRI_ASSERT(ServerState::instance()->isSingleServer() && !isFollowerTransaction || + TRI_ASSERT((ServerState::instance()->isSingleServer() && !isFollowerTransaction) || tid.isFollowerTransactionId() == isFollowerTransaction); transaction::Options options; std::vector reads, writes, exclusives; From 871357f0f405cea61e4bcee19eef683dff91225a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Tue, 26 Oct 2021 10:15:46 +0200 Subject: [PATCH 10/15] Fixed remaining merge conflicts --- arangod/RocksDBEngine/RocksDBTransactionState.cpp | 6 +----- arangod/StorageEngine/TransactionState.cpp | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/arangod/RocksDBEngine/RocksDBTransactionState.cpp b/arangod/RocksDBEngine/RocksDBTransactionState.cpp index 5078fed59087..af4513a365b8 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionState.cpp +++ b/arangod/RocksDBEngine/RocksDBTransactionState.cpp @@ -351,7 +351,7 @@ RocksDBTransactionState* RocksDBTransactionState::toState(transaction::Methods* return static_cast(state); } -RocksDBTransactionMethods* RocksDBTransactionState::toMethods(transaction::Methods* trx) { +RocksDBTransactionMethods* RocksDBTransactionState::toMethods(transaction::Methods* trx, DataSourceId collectionId) { TRI_ASSERT(trx != nullptr); TransactionState* state = trx->state(); TRI_ASSERT(state != nullptr); @@ -360,10 +360,6 @@ RocksDBTransactionMethods* RocksDBTransactionState::toMethods(transaction::Metho void RocksDBTransactionState::prepareForParallelReads() { _parallel = true; } bool RocksDBTransactionState::inParallelMode() const { return _parallel; } -RocksDBTransactionMethods* RocksDBTransactionState::rocksdbMethods() { - TRI_ASSERT(_rocksMethods); - return _rocksMethods.get(); -} #ifdef ARANGODB_ENABLE_MAINTAINER_MODE RocksDBTransactionStateGuard::RocksDBTransactionStateGuard(RocksDBTransactionState* state) noexcept diff --git a/arangod/StorageEngine/TransactionState.cpp b/arangod/StorageEngine/TransactionState.cpp index 0a01a9d9512c..77c9212190dc 100644 --- a/arangod/StorageEngine/TransactionState.cpp +++ b/arangod/StorageEngine/TransactionState.cpp @@ -244,7 +244,7 @@ Result TransactionState::addCollectionInternal(DataSourceId cid, std::string con // collection was not contained. now create and insert it - auto* const trxColl = engine.createTransactionCollection(*this, cid, accessType).release(); + auto* const trxColl = createTransactionCollection(cid, accessType).release(); TRI_ASSERT(trxColl != nullptr); From cea7b787f08770d25fb4b5f681e1c11180534e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 28 Oct 2021 10:26:48 +0200 Subject: [PATCH 11/15] Added comments, assertions, and moved initializations in transaction::Options from member initializer list to default member initializers --- arangod/Transaction/Manager.cpp | 9 +++++++++ arangod/Transaction/Options.cpp | 20 +------------------- arangod/Transaction/Options.h | 32 +++++++++++++++++--------------- 3 files changed, 27 insertions(+), 34 deletions(-) diff --git a/arangod/Transaction/Manager.cpp b/arangod/Transaction/Manager.cpp index 7df0e803d7ec..df2a4f6c7051 100644 --- a/arangod/Transaction/Manager.cpp +++ b/arangod/Transaction/Manager.cpp @@ -628,6 +628,15 @@ Result Manager::ensureManagedTrx(TRI_vocbase_t& vocbase, TransactionId tid, return res.reset(TRI_ERROR_SHUTTING_DOWN); } + // This method should not be used in a single server. Note that single-server + // transaction IDs will randomly be identified as follower transactions, + // leader transactions, legacy transactions or coordinator transactions; + // context is important. + TRI_ASSERT(!ServerState::instance()->isSingleServer()); + // We should never have `options.isFollowerTransaction == true`, but + // `tid.isFollowerTransactionId() == false`. + TRI_ASSERT(options.isFollowerTransaction == tid.isFollowerTransactionId() || + !options.isFollowerTransaction); options.isFollowerTransaction = tid.isFollowerTransactionId(); LOG_TOPIC("7bd2d", DEBUG, Logger::TRANSACTIONS) diff --git a/arangod/Transaction/Options.cpp b/arangod/Transaction/Options.cpp index 0a7893203fc5..51034fb5a379 100644 --- a/arangod/Transaction/Options.cpp +++ b/arangod/Transaction/Options.cpp @@ -32,25 +32,7 @@ using namespace arangodb::transaction; -uint64_t Options::defaultMaxTransactionSize = UINT64_MAX; -uint64_t Options::defaultIntermediateCommitSize = 512 * 1024 * 1024; -uint64_t Options::defaultIntermediateCommitCount = 1 * 1000 * 1000; - -Options::Options() - : lockTimeout(defaultLockTimeout), - maxTransactionSize(defaultMaxTransactionSize), - intermediateCommitSize(defaultIntermediateCommitSize), - intermediateCommitCount(defaultIntermediateCommitCount), - allowImplicitCollectionsForRead(true), - allowImplicitCollectionsForWrite(false), -#ifdef USE_ENTERPRISE - skipInaccessibleCollections(false), -#endif - waitForSync(false), - fillBlockCache(true), - isFollowerTransaction(false), - origin("", arangodb::RebootId(0)) { - +Options::Options() { // if we are a coordinator, fill in our own server id/reboot id. // the data is passed to DB servers when the transaction is started // there. the DB servers use this data to abort the transaction diff --git a/arangod/Transaction/Options.h b/arangod/Transaction/Options.h index 7d221148c79c..74ad0f3ff710 100644 --- a/arangod/Transaction/Options.h +++ b/arangod/Transaction/Options.h @@ -60,23 +60,25 @@ struct Options { bool isIntermediateCommitEnabled() const noexcept; static constexpr double defaultLockTimeout = 900.0; - static uint64_t defaultMaxTransactionSize; - static uint64_t defaultIntermediateCommitSize; - static uint64_t defaultIntermediateCommitCount; + static constexpr std::uint64_t defaultMaxTransactionSize = + std::numeric_limits::max(); + static constexpr std::uint64_t defaultIntermediateCommitSize = + std::uint64_t{1} << 29U; // 512 * 1024 * 1024 + static constexpr std::uint64_t defaultIntermediateCommitCount = 1'000'000; /// @brief time (in seconds) that is spent waiting for a lock - double lockTimeout; - uint64_t maxTransactionSize; - uint64_t intermediateCommitSize; - uint64_t intermediateCommitCount; - bool allowImplicitCollectionsForRead; - bool allowImplicitCollectionsForWrite; // replication only! + double lockTimeout = defaultLockTimeout; + std::uint64_t maxTransactionSize = defaultMaxTransactionSize; + std::uint64_t intermediateCommitSize = defaultIntermediateCommitSize; + std::uint64_t intermediateCommitCount = defaultIntermediateCommitCount; + bool allowImplicitCollectionsForRead = true; + bool allowImplicitCollectionsForWrite = false; // replication only! #ifdef USE_ENTERPRISE - bool skipInaccessibleCollections; + bool skipInaccessibleCollections = false; #endif - bool waitForSync; - bool fillBlockCache; - bool isFollowerTransaction; + bool waitForSync = false; + bool fillBlockCache = true; + bool isFollowerTransaction = false; /// @brief originating server of this transaction. will be populated /// only in the cluster, and with a coordinator id/coordinator reboot id @@ -86,12 +88,12 @@ struct Options { /// abort the transaction should the coordinator die or be rebooted. /// the server id and reboot id are intentionally empty in single server /// case. - arangodb::cluster::RebootTracker::PeerState origin; + arangodb::cluster::RebootTracker::PeerState origin = {"", arangodb::RebootId(0)}; }; struct AllowImplicitCollectionsSwitcher { AllowImplicitCollectionsSwitcher(Options& options, bool allow) noexcept - : _options(options), + : _options(options), _oldValue(options.allowImplicitCollectionsForRead) { // previous value has been saved, now override value in options with disallow options.allowImplicitCollectionsForRead = allow; From d4963ffb6136eee84eb59fc906fd36252578f49b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 28 Oct 2021 13:07:23 +0200 Subject: [PATCH 12/15] Fixed compile error --- arangod/Transaction/Options.cpp | 8 ++++++++ arangod/Transaction/Options.h | 8 +++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/arangod/Transaction/Options.cpp b/arangod/Transaction/Options.cpp index 51034fb5a379..457a9e6ca153 100644 --- a/arangod/Transaction/Options.cpp +++ b/arangod/Transaction/Options.cpp @@ -32,6 +32,14 @@ using namespace arangodb::transaction; +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +uint64_t Options::defaultMaxTransactionSize = + std::numeric_limits::max(); +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +uint64_t Options::defaultIntermediateCommitSize = std::uint64_t{512} * 1024 * 1024; // 1 << 29 +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +uint64_t Options::defaultIntermediateCommitCount = 1'000'000; + Options::Options() { // if we are a coordinator, fill in our own server id/reboot id. // the data is passed to DB servers when the transaction is started diff --git a/arangod/Transaction/Options.h b/arangod/Transaction/Options.h index 74ad0f3ff710..e7ff7f9a1440 100644 --- a/arangod/Transaction/Options.h +++ b/arangod/Transaction/Options.h @@ -60,11 +60,9 @@ struct Options { bool isIntermediateCommitEnabled() const noexcept; static constexpr double defaultLockTimeout = 900.0; - static constexpr std::uint64_t defaultMaxTransactionSize = - std::numeric_limits::max(); - static constexpr std::uint64_t defaultIntermediateCommitSize = - std::uint64_t{1} << 29U; // 512 * 1024 * 1024 - static constexpr std::uint64_t defaultIntermediateCommitCount = 1'000'000; + static std::uint64_t defaultMaxTransactionSize; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + static std::uint64_t defaultIntermediateCommitSize; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + static std::uint64_t defaultIntermediateCommitCount; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) /// @brief time (in seconds) that is spent waiting for a lock double lockTimeout = defaultLockTimeout; From 39db31021edbcdb423af5a9278e0543260d3165f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Thu, 28 Oct 2021 17:30:22 +0200 Subject: [PATCH 13/15] [WIP] Try to fix google test error --- arangod/Cluster/ServerState.cpp | 6 ++++++ arangod/Cluster/ServerState.h | 11 +++++++++++ arangod/Transaction/Manager.cpp | 3 ++- arangod/VocBase/Identifiers/TransactionId.cpp | 8 ++++++++ arangod/VocBase/Identifiers/TransactionId.h | 4 ++++ tests/Transaction/Manager-test.cpp | 4 ++-- tests/main.cpp | 1 + 7 files changed, 34 insertions(+), 3 deletions(-) diff --git a/arangod/Cluster/ServerState.cpp b/arangod/Cluster/ServerState.cpp index f9dbf8594281..228248b02c72 100644 --- a/arangod/Cluster/ServerState.cpp +++ b/arangod/Cluster/ServerState.cpp @@ -1190,3 +1190,9 @@ Result ServerState::propagateClusterReadOnly(bool mode) { setReadOnly(mode ? API_TRUE : API_FALSE); return Result(); } + +bool ServerState::isGoogleTest() const noexcept { return _isGoogleTests; } + +void ServerState::setGoogleTest(bool isGoogleTests) noexcept { + _isGoogleTests = isGoogleTests; +} diff --git a/arangod/Cluster/ServerState.h b/arangod/Cluster/ServerState.h index 37c5341c334c..4deede69d638 100644 --- a/arangod/Cluster/ServerState.h +++ b/arangod/Cluster/ServerState.h @@ -282,6 +282,13 @@ class ServerState { /// file where the server persists its UUID std::string getUuidFilename() const; +#ifdef ARANGODB_USE_GOOGLE_TESTS + [[nodiscard]] bool isGoogleTest() const noexcept; + void setGoogleTest(bool isGoogleTests) noexcept; +#else + [[nodiscard]] constexpr bool isGoogleTest() const noexcept { return false; } +#endif + private: /// @brief atomically fetches the server role inline RoleEnum loadRole() const noexcept { @@ -374,6 +381,10 @@ class ServerState { TRI_voc_tick_t _foxxmasterSince; bool _foxxmasterQueueupdate; + +#ifdef ARANGODB_USE_GOOGLE_TESTS + bool _isGoogleTests = false; +#endif }; } // namespace arangodb diff --git a/arangod/Transaction/Manager.cpp b/arangod/Transaction/Manager.cpp index df2a4f6c7051..9ec627b4e3f3 100644 --- a/arangod/Transaction/Manager.cpp +++ b/arangod/Transaction/Manager.cpp @@ -632,7 +632,8 @@ Result Manager::ensureManagedTrx(TRI_vocbase_t& vocbase, TransactionId tid, // transaction IDs will randomly be identified as follower transactions, // leader transactions, legacy transactions or coordinator transactions; // context is important. - TRI_ASSERT(!ServerState::instance()->isSingleServer()); + TRI_ASSERT(!ServerState::instance()->isSingleServer() || + ServerState::instance()->isGoogleTest()); // We should never have `options.isFollowerTransaction == true`, but // `tid.isFollowerTransactionId() == false`. TRI_ASSERT(options.isFollowerTransaction == tid.isFollowerTransactionId() || diff --git a/arangod/VocBase/Identifiers/TransactionId.cpp b/arangod/VocBase/Identifiers/TransactionId.cpp index c74ac240d62a..a61a6d4097b5 100644 --- a/arangod/VocBase/Identifiers/TransactionId.cpp +++ b/arangod/VocBase/Identifiers/TransactionId.cpp @@ -62,4 +62,12 @@ TransactionId TransactionId::createLegacy() { return TransactionId(TRI_NewServerSpecificTickMod4() + 3); } +TransactionId TransactionId::createLeader() { + return TransactionId(TRI_NewServerSpecificTickMod4() + 1); +} + +TransactionId TransactionId::createFollower() { + return TransactionId(TRI_NewServerSpecificTickMod4() + 2); +} + } // namespace arangodb diff --git a/arangod/VocBase/Identifiers/TransactionId.h b/arangod/VocBase/Identifiers/TransactionId.h index c61392005aa3..8e67e0e74a0d 100644 --- a/arangod/VocBase/Identifiers/TransactionId.h +++ b/arangod/VocBase/Identifiers/TransactionId.h @@ -64,6 +64,10 @@ class TransactionId : public basics::Identifier { /// @brief create a legacy id static TransactionId createLegacy(); + + static TransactionId createLeader(); + + static TransactionId createFollower(); }; // TransactionId should not be bigger than the BaseType diff --git a/tests/Transaction/Manager-test.cpp b/tests/Transaction/Manager-test.cpp index c6f9e9e8da60..876ebf273f52 100644 --- a/tests/Transaction/Manager-test.cpp +++ b/tests/Transaction/Manager-test.cpp @@ -76,7 +76,7 @@ class TransactionManagerTest : public ::testing::Test { TransactionManagerTest() : vocbase(TRI_vocbase_type_e::TRI_VOCBASE_TYPE_NORMAL, testDBInfo(setup.server.server())), mgr(transaction::ManagerFeature::manager()), - tid(TRI_NewTickServer()) {} + tid(TransactionId::createLeader()) {} ~TransactionManagerTest() { mgr->garbageCollect(true); } }; @@ -245,7 +245,7 @@ TEST_F(TransactionManagerTest, simple_transaction_and_commit_is_follower) { auto json = arangodb::velocypack::Parser::fromJson( "{ \"collections\":{\"write\": [\"42\"]}}"); - Result res = mgr->ensureManagedTrx(vocbase, tid, json->slice(), true); + Result res = mgr->ensureManagedTrx(vocbase, TransactionId::createFollower(), json->slice(), true); ASSERT_TRUE(res.ok()); { diff --git a/tests/main.cpp b/tests/main.cpp index b3fdc71cd265..f743dca8051b 100644 --- a/tests/main.cpp +++ b/tests/main.cpp @@ -132,6 +132,7 @@ int main(int argc, char* argv[]) { // many other places rely on the reboot id being initialized, // so we do it here in a central place arangodb::ServerState::instance()->setRebootId(arangodb::RebootId{1}); + arangodb::ServerState::instance()->setGoogleTest(true); IcuInitializer::setup(ARGV0); // enable mocking globally - not awesome, but helps to prevent runtime From a9718ffe223e8ae6c3946aa5eff6035e89837afd Mon Sep 17 00:00:00 2001 From: mpoeter Date: Fri, 29 Oct 2021 14:35:37 +0200 Subject: [PATCH 14/15] Fix test --- tests/Transaction/Manager-test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Transaction/Manager-test.cpp b/tests/Transaction/Manager-test.cpp index 876ebf273f52..72ec510b2582 100644 --- a/tests/Transaction/Manager-test.cpp +++ b/tests/Transaction/Manager-test.cpp @@ -230,6 +230,7 @@ TEST_F(TransactionManagerTest, simple_transaction_and_commit) { } TEST_F(TransactionManagerTest, simple_transaction_and_commit_is_follower) { + tid = TransactionId::createFollower(); auto beforeRole = arangodb::ServerState::instance()->getRole(); auto roleGuard = scopeGuard([&]() noexcept { arangodb::ServerState::instance()->setRole(beforeRole); @@ -245,7 +246,7 @@ TEST_F(TransactionManagerTest, simple_transaction_and_commit_is_follower) { auto json = arangodb::velocypack::Parser::fromJson( "{ \"collections\":{\"write\": [\"42\"]}}"); - Result res = mgr->ensureManagedTrx(vocbase, TransactionId::createFollower(), json->slice(), true); + Result res = mgr->ensureManagedTrx(vocbase, tid, json->slice(), true); ASSERT_TRUE(res.ok()); { From 66a8cd5f5483867703fca2fccdaf88740abdc013 Mon Sep 17 00:00:00 2001 From: mpoeter Date: Tue, 2 Nov 2021 13:33:03 +0100 Subject: [PATCH 15/15] Fix non-maintainer build. --- arangod/Cluster/ServerState.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arangod/Cluster/ServerState.cpp b/arangod/Cluster/ServerState.cpp index 228248b02c72..2c1b5d41253a 100644 --- a/arangod/Cluster/ServerState.cpp +++ b/arangod/Cluster/ServerState.cpp @@ -1191,8 +1191,10 @@ Result ServerState::propagateClusterReadOnly(bool mode) { return Result(); } +#ifdef ARANGODB_USE_GOOGLE_TESTS bool ServerState::isGoogleTest() const noexcept { return _isGoogleTests; } void ServerState::setGoogleTest(bool isGoogleTests) noexcept { _isGoogleTests = isGoogleTests; } +#endif