From b91eab0ce85382344079ed58a0c5b9528495eb3a Mon Sep 17 00:00:00 2001 From: jsteemann Date: Thu, 1 Jun 2017 02:01:33 +0200 Subject: [PATCH] honor transaction options --- arangod/Aql/AqlTransaction.cpp | 7 +++++++ arangod/Aql/AqlTransaction.h | 12 +++-------- arangod/Aql/Query.cpp | 21 +++++++++++++------ arangod/Aql/QueryOptions.cpp | 2 +- arangod/Cluster/TraverserEngine.cpp | 2 +- arangod/RestHandler/RestQueryHandler.cpp | 4 ++-- .../RocksDBEngine/RocksDBTransactionState.cpp | 15 ++++++------- arangod/StorageEngine/TransactionState.h | 2 ++ arangod/Transaction/Methods.cpp | 4 +--- arangod/Transaction/Methods.h | 2 +- arangod/Transaction/V8Context.cpp | 16 +++++++++----- arangod/Transaction/V8Context.h | 5 ++++- arangod/V8Server/v8-vocbaseprivate.h | 2 +- 13 files changed, 57 insertions(+), 37 deletions(-) diff --git a/arangod/Aql/AqlTransaction.cpp b/arangod/Aql/AqlTransaction.cpp index bc91c4a579..a004c95847 100644 --- a/arangod/Aql/AqlTransaction.cpp +++ b/arangod/Aql/AqlTransaction.cpp @@ -31,6 +31,13 @@ using namespace arangodb; using namespace arangodb::aql; +/// @brief clone, used to make daughter transactions for parts of a +/// distributed AQL query running on the coordinator +transaction::Methods* AqlTransaction::clone(transaction::Options const& options) const { + return new AqlTransaction(transaction::StandaloneContext::Create(vocbase()), + &_collections, options, false); +} + /// @brief add a collection to the transaction Result AqlTransaction::processCollection(aql::Collection* collection) { if (ServerState::instance()->isCoordinator()) { diff --git a/arangod/Aql/AqlTransaction.h b/arangod/Aql/AqlTransaction.h index 43b834954e..18be79e8db 100644 --- a/arangod/Aql/AqlTransaction.h +++ b/arangod/Aql/AqlTransaction.h @@ -45,8 +45,7 @@ class AqlTransaction final : public transaction::Methods { transaction::Options const& options, bool isMainTransaction) : transaction::Methods(transactionContext, options), - _collections(*collections), - _options(options) { + _collections(*collections) { if (!isMainTransaction) { addHint(transaction::Hints::Hint::LOCK_NEVER); } else { @@ -90,12 +89,8 @@ class AqlTransaction final : public transaction::Methods { LogicalCollection* documentCollection(TRI_voc_cid_t cid); /// @brief clone, used to make daughter transactions for parts of a - /// distributed - /// AQL query running on the coordinator - transaction::Methods* clone() const override { - return new AqlTransaction(transaction::StandaloneContext::Create(vocbase()), - &_collections, _options, false); - } + /// distributed AQL query running on the coordinator + transaction::Methods* clone(transaction::Options const&) const override; /// @brief lockCollections, this is needed in a corner case in AQL: we need /// to lock all shards in a controlled way when we set up a distributed @@ -109,7 +104,6 @@ class AqlTransaction final : public transaction::Methods { /// operation private: std::map _collections; - transaction::Options _options; }; } diff --git a/arangod/Aql/Query.cpp b/arangod/Aql/Query.cpp index 997a2840f3..97d3cc5701 100644 --- a/arangod/Aql/Query.cpp +++ b/arangod/Aql/Query.cpp @@ -100,6 +100,14 @@ Query::Query(bool contextOwnedByExterior, TRI_vocbase_t* vocbase, if (aql == nullptr) { THROW_ARANGO_EXCEPTION(TRI_ERROR_SHUTTING_DOWN); } + + if (_contextOwnedByExterior) { + // copy transaction options from global state into our local query options + TransactionState* state = transaction::V8Context::getParentState(); + if (state != nullptr) { + _queryOptions.transactionOptions = state->options(); + } + } // populate query options if (_options != nullptr) { @@ -248,8 +256,9 @@ Query* Query::clone(QueryPart part, bool withPlan) { TRI_ASSERT(clone->_trx == nullptr); - clone->_trx = _trx->clone(); // A daughter transaction which does not - // actually lock the collections + // A daughter transaction which does not + // actually lock the collections + clone->_trx = _trx->clone(_queryOptions.transactionOptions); Result res = clone->_trx->begin(); @@ -418,12 +427,12 @@ ExecutionPlan* Query::prepare() { std::unique_ptr plan; if (!_queryString.empty()) { - auto parser = std::make_unique(this); + Parser parser(this); - parser->parse(false); + parser.parse(false); // put in bind parameters - parser->ast()->injectBindParameters(_bindParameters); - _isModificationQuery = parser->isModificationQuery(); + parser.ast()->injectBindParameters(_bindParameters); + _isModificationQuery = parser.isModificationQuery(); } TRI_ASSERT(_trx == nullptr); diff --git a/arangod/Aql/QueryOptions.cpp b/arangod/Aql/QueryOptions.cpp index abc852cd5e..39eef136d8 100644 --- a/arangod/Aql/QueryOptions.cpp +++ b/arangod/Aql/QueryOptions.cpp @@ -68,7 +68,7 @@ QueryOptions::QueryOptions() : auto queryCacheMode = QueryCache::instance()->mode(); cache = (queryCacheMode == CACHE_ALWAYS_ON); } - + void QueryOptions::fromVelocyPack(VPackSlice const& slice) { if (!slice.isObject()) { return; diff --git a/arangod/Cluster/TraverserEngine.cpp b/arangod/Cluster/TraverserEngine.cpp index 3988642b7a..ecfce46aaf 100644 --- a/arangod/Cluster/TraverserEngine.cpp +++ b/arangod/Cluster/TraverserEngine.cpp @@ -106,7 +106,7 @@ BaseEngine::BaseEngine(TRI_vocbase_t* vocbase, VPackSlice info) // created transaction is considered a "MAIN" part and will not switch // off collection locking completely! _query = - new aql::Query(true, vocbase, aql::QueryString(), params, opts, aql::PART_DEPENDENT); + new aql::Query(false, vocbase, aql::QueryString(), params, opts, aql::PART_DEPENDENT); _query->injectTransaction(_trx); VPackSlice variablesSlice = info.get(VARIABLES); diff --git a/arangod/RestHandler/RestQueryHandler.cpp b/arangod/RestHandler/RestQueryHandler.cpp index 9901fa0832..46c8e3f684 100644 --- a/arangod/RestHandler/RestQueryHandler.cpp +++ b/arangod/RestHandler/RestQueryHandler.cpp @@ -306,10 +306,10 @@ bool RestQueryHandler::parseQuery() { "expecting a JSON object as body"); }; - std::string const&& queryString = + std::string const queryString = VelocyPackHelper::checkAndGetStringValue(body, "query"); - Query query(true, _vocbase, QueryString(queryString), + Query query(false, _vocbase, QueryString(queryString), nullptr, nullptr, PART_MAIN); auto parseResult = query.parse(); diff --git a/arangod/RocksDBEngine/RocksDBTransactionState.cpp b/arangod/RocksDBEngine/RocksDBTransactionState.cpp index f8cb56bee7..b68cd1b433 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionState.cpp +++ b/arangod/RocksDBEngine/RocksDBTransactionState.cpp @@ -438,15 +438,16 @@ RocksDBOperationResult RocksDBTransactionState::addOperation( // "transaction size" counters have reached their limit if (_options.intermediateCommitCount <= numOperations || _options.intermediateCommitSize <= newSize) { - internalCommit(); - _numInserts = 0; - _numUpdates = 0; - _numRemoves = 0; + // LOG_TOPIC(ERR, Logger::FIXME) << "INTERMEDIATE COMMIT!"; + internalCommit(); + _numInserts = 0; + _numUpdates = 0; + _numRemoves = 0; #ifdef ARANGODB_ENABLE_MAINTAINER_MODE - _numLogdata = 0; + _numLogdata = 0; #endif - createTransaction(); - } + createTransaction(); + } return res; } diff --git a/arangod/StorageEngine/TransactionState.h b/arangod/StorageEngine/TransactionState.h index 16c2d0ee1f..a9da6bc5d7 100644 --- a/arangod/StorageEngine/TransactionState.h +++ b/arangod/StorageEngine/TransactionState.h @@ -68,6 +68,8 @@ class TransactionState { bool isDBServer() const { return ServerState::isDBServer(_serverRole); } bool isCoordinator() const { return ServerState::isCoordinator(_serverRole); } + transaction::Options& options() { return _options; } + transaction::Options const& options() const { return _options; } TRI_vocbase_t* vocbase() const { return _vocbase; } TRI_voc_tid_t id() const { return _id; } transaction::Status status() const { return _status; } diff --git a/arangod/Transaction/Methods.cpp b/arangod/Transaction/Methods.cpp index 1de4336800..24b43a1475 100644 --- a/arangod/Transaction/Methods.cpp +++ b/arangod/Transaction/Methods.cpp @@ -2701,14 +2701,12 @@ std::vector> transaction::Methods::indexesForCollection( } /// @brief Lock all collections. Only works for selected sub-classes - int transaction::Methods::lockCollections() { THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED); } /// @brief Clone this transaction. Only works for selected sub-classes - -transaction::Methods* transaction::Methods::clone() const { +transaction::Methods* transaction::Methods::clone(transaction::Options const&) const { THROW_ARANGO_EXCEPTION(TRI_ERROR_NOT_IMPLEMENTED); } diff --git a/arangod/Transaction/Methods.h b/arangod/Transaction/Methods.h index cbceb9bd2b..dd9d0b8cc6 100644 --- a/arangod/Transaction/Methods.h +++ b/arangod/Transaction/Methods.h @@ -373,7 +373,7 @@ class Methods { virtual int lockCollections(); /// @brief Clone this transaction. Only works for selected sub-classes - virtual transaction::Methods* clone() const; + virtual transaction::Methods* clone(transaction::Options const&) const; /// @brief return the collection name resolver CollectionNameResolver const* resolver() const; diff --git a/arangod/Transaction/V8Context.cpp b/arangod/Transaction/V8Context.cpp index 14e1f65ed4..631ca5a3bc 100644 --- a/arangod/Transaction/V8Context.cpp +++ b/arangod/Transaction/V8Context.cpp @@ -115,15 +115,21 @@ bool transaction::V8Context::isGlobal() const { return _sharedTransactionContext == this; } -/// @brief check whether the transaction is embedded -bool transaction::V8Context::IsEmbedded() { +/// @brief return parent transaction state or none +TransactionState* transaction::V8Context::getParentState() { TRI_v8_global_t* v8g = static_cast( v8::Isolate::GetCurrent()->GetData(V8PlatformFeature::V8_DATA_SLOT)); - if (v8g->_transactionContext == nullptr) { - return false; + if (v8g == nullptr || + v8g->_transactionContext == nullptr) { + return nullptr; } return static_cast(v8g->_transactionContext) - ->_currentTransaction != nullptr; + ->_currentTransaction; +} + +/// @brief check whether the transaction is embedded +bool transaction::V8Context::isEmbedded() { + return (getParentState() != nullptr); } /// @brief create a context, returned in a shared ptr diff --git a/arangod/Transaction/V8Context.h b/arangod/Transaction/V8Context.h index 7320fa5233..fb90d45635 100644 --- a/arangod/Transaction/V8Context.h +++ b/arangod/Transaction/V8Context.h @@ -69,8 +69,11 @@ class V8Context final : public Context { /// @brief whether or not the transaction context is a global one bool isGlobal() const; + /// @brief return parent transaction state or none + static TransactionState* getParentState(); + /// @brief check whether the transaction is embedded - static bool IsEmbedded(); + static bool isEmbedded(); /// @brief create a context, returned in a shared ptr static std::shared_ptr Create(TRI_vocbase_t*, bool); diff --git a/arangod/V8Server/v8-vocbaseprivate.h b/arangod/V8Server/v8-vocbaseprivate.h index a1eb2fb0bb..ac02c4d433 100644 --- a/arangod/V8Server/v8-vocbaseprivate.h +++ b/arangod/V8Server/v8-vocbaseprivate.h @@ -36,7 +36,7 @@ //////////////////////////////////////////////////////////////////////////////// #define PREVENT_EMBEDDED_TRANSACTION() \ - if (arangodb::transaction::V8Context::IsEmbedded()) { \ + if (arangodb::transaction::V8Context::isEmbedded()) { \ TRI_V8_THROW_EXCEPTION(TRI_ERROR_TRANSACTION_DISALLOWED_OPERATION); \ }