8000 Make all synchronous transaction methods set skipScheduler for network requests by goedderz · Pull Request #14810 · arangodb/arangodb · GitHub
[go: up one dir, main page]
Skip to content

Make all synchronous transaction methods set skipScheduler for network requests #14810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Sep 23, 2021
Merged

Make all synchronous transaction methods set skipScheduler for network requests #14810

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4f8f935
Moved code from transaction::Methods::*Async() methods to -Internal m…
goedderz Aug 30, 2021
5c4f479
Don't go back to *Async calls from *Internal
goedderz Aug 30, 2021
cf73e0e
Use -Internal methods for synchronous API as well
goedderz Aug 30, 2021
fcc030d
Updated comments
goedderz Aug 30, 2021
5c84f5b
Make count- and documentInternal virtual, instead of count and document
goedderz Aug 31, 2021
b1a33e5
Wrote a separate TODO note per method
goedderz Aug 31, 2021
9d290d6
Moved Methods::Api to MethodsApi
goedderz Aug 31, 2021
b8b4058
Set skipScheduler in commit-, finish-, and abortInternal
goedderz Aug 31, 2021
23514f8
Set skipScheduler in countInternal
goedderz Sep 21, 2021
32b08de
Set skipScheduler in documentInternal
goedderz Sep 21, 2021
430110f
Set skipScheduler in truncateInternal
goedderz Sep 21, 2021
a21ce89
Set skipScheduler in insertInternal
goedderz Sep 21, 2021
e1b2c9e
Set skipScheduler in update- and replaceInternal
goedderz Sep 21, 2021
786bfa3
Set skipScheduler in removeInternal
goedderz Sep 21, 2021
9529fd4
Merge branch 'devel' of github.com:arangodb/arangodb into bug-fix/fix…
goedderz Sep 21, 2021
ca414ab
Fixed some overlooked places
goedderz Sep 22, 2021
fd27adb
Added CHANGELOG entry
goedderz Sep 22, 2021
b6b3700
Changed CHANGELOG entry to be more user-friendly
goedderz Sep 22, 2021
cf9fcef
Merge branch 'devel' into bug-fix/fix-coordinator-aql-request-priorities
jsteemann Sep 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Moved Methods::Api to MethodsApi
  • Loading branch information
@goedderz
goedderz committed Aug 31, 2021
commit 9d290d6115e08173289b45c8e262d4a64fe8680e
54 changes: 27 additions & 27 deletions arangod/Transaction/Methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,17 +510,17 @@ Result transaction::Methods::begin() {

/// @brief commit / finish the transaction
Future<Result> transaction::Methods::commitAsync() {
return commitInternal(Api::Asynchronous);
return commitInternal(MethodsApi::Asynchronous);
}

/// @brief abort the transaction
Future<Result> transaction::Methods::abortAsync() {
return abortInternal(Api::Asynchronous);
return abortInternal(MethodsApi::Asynchronous);
}

/// @brief finish a transaction (commit or abort), based on the previous state
Future<Result> transaction::Methods::finishAsync(Result const& res) {
return finishInternal(res, Api::Asynchronous);
return finishInternal(res, MethodsApi::Asynchronous);
}

/// @brief return the transaction id
Expand Down Expand Up @@ -759,14 +759,14 @@ Future<OperationResult> addTracking(Future<OperationResult>&& f, F&& func) {
OperationResult Methods::document(std::string const& collectionName,
VPackSlice value,
OperationOptions const& options) {
return documentInternal(collectionName, value, options, Api::Synchronous).get();
return documentInternal(collectionName, value, options, MethodsApi::Synchronous).get();
}

/// @brief return one or multiple documents from a collection
Future<OperationResult> transaction::Methods::documentAsync(std::string const& cname,
VPackSlice value,
OperationOptions const& options) {
return documentInternal(cname, value, options, Api::Asynchronous);
return documentInternal(cname, value, options, MethodsApi::Asynchronous);
}

/// @brief read one or multiple documents in a collection, coordinator
Expand Down Expand Up @@ -872,7 +872,7 @@ Future<OperationResult> transaction::Methods::documentLocal(std::string const& c

OperationResult Methods::insert(std::string const& cname, VPackSlice value,
OperationOptions const& options) {
return insertInternal(cname, value, options, Api::Synchronous).get();
return insertInternal(cname, value, options, MethodsApi::Synchronous).get();
}

/// @brief create one or multiple documents in a collection
Expand All @@ -881,7 +881,7 @@ OperationResult Methods::insert(std::string const& cname, VPackSlice value,
Future<OperationResult> transaction::Methods::insertAsync(std::string const& cname,
VPackSlice value,
OperationOptions const& options) {
return insertInternal(cname, value, options, Api::Asynchronous);
return insertInternal(cname, value, options, MethodsApi::Asynchronous);
}

/// @brief create one or multiple documents in a collection, coordinator
Expand Down Expand Up @@ -1175,7 +1175,7 @@ Future<OperationResult> transaction::Methods::insertLocal(std::string const& cna

OperationResult Methods::update(std::string const& cname, VPackSlice updateValue,
OperationOptions const& options) {
return updateInternal(cname, updateValue, options, Api::Synchronous).get();
return updateInternal(cname, updateValue, options, MethodsApi::Synchronous).get();
}

/// @brief update/patch one or multiple documents in a collection
Expand All @@ -1184,7 +1184,7 @@ OperationResult Methods::update(std::string const& cname, VPackSlice updateValue
Future<OperationResult> transaction::Methods::updateAsync(std::string const& cname,
VPackSlice newValue,
OperationOptions const& options) {
return updateInternal(cname, newValue, options, Api::Asynchronous);
return updateInternal(cname, newValue, options, MethodsApi::Asynchronous);
}

/// @brief update one or multiple documents in a collection, coordinator
Expand Down Expand Up @@ -1213,7 +1213,7 @@ Future<OperationResult> transaction::Methods::modifyCoordinator(

OperationResult Methods::replace(std::string const& cname, VPackSlice replaceValue,
OperationOptions const& options) {
return replaceInternal(cname, replaceValue, options, Api::Synchronous).get();
return replaceInternal(cname, replaceValue, options, MethodsApi::Synchronous).get();
}

/// @brief replace one or multiple documents in a collection
Expand All @@ -1222,7 +1222,7 @@ OperationResult Methods::replace(std::string const& cname, VPackSlice replaceVal
Future<OperationResult> transaction::Methods::replaceAsync(std::string const& cname,
VPackSlice newValue,
OperationOptions const& options) {
return replaceInternal(cname, newValue, options, Api::Asynchronous);
return replaceInternal(cname, newValue, options, MethodsApi::Asynchronous);
}

/// @brief replace one or multiple documents in a collection, local
Expand Down Expand Up @@ -1411,7 +1411,7 @@ Future<OperationResult> transaction::Methods::modifyLocal(std::string const& col

OperationResult Methods::remove(std::string const& collectionName,
VPackSlice value, OperationOptions const& options) {
return removeInternal(collectionName, value, options, Api::Synchronous).get();
return removeInternal(collectionName, value, options, MethodsApi::Synchronous).get();
}

/// @brief remove one or multiple documents in a collection
Expand All @@ -1420,7 +1420,7 @@ OperationResult Methods::remove(std::string const& collectionName,
Future<OperationResult> transaction::Methods::removeAsync(std::string const& cname,
VPackSlice value,
OperationOptions const& options) {
return removeInternal(cname, value, options, Api::Asynchronous);
return removeInternal(cname, value, options, MethodsApi::Asynchronous);
}

/// @brief remove one or multiple documents in a collection, coordinator
Expand Down Expand Up @@ -1662,13 +1662,13 @@ OperationResult transaction::Methods::allLocal(std::string const& collectionName

OperationResult Methods::truncate(std::string const& collectionName,
OperationOptions const& options) {
return truncateInternal(collectionName, options, Api::Synchronous).get();
return truncateInternal(collectionName, options, MethodsApi::Synchronous).get();
}

/// @brief remove all documents in a collection
Future<OperationResult> transaction::Methods::truncateAsync(std::string const& collectionName,
OperationOptions const& options) {
return truncateInternal(collectionName, options, Api::Asynchronous);
return truncateInternal(collectionName, options, MethodsApi::Asynchronous);
}

/// @brief remove all documents in a collection, coordinator
Expand Down Expand Up @@ -1851,14 +1851,14 @@ Future<OperationResult> transaction::Methods::truncateLocal(std::string const& c

OperationResult Methods::count(std::string const& collectionName,
CountType type, OperationOptions const& options) {
return countInternal(collectionName, type, options, Api::Synchronous).get();
return countInternal(collectionName, type, options, MethodsApi::Synchronous).get();
}

/// @brief count the number of documents in a collection
futures::Future<OperationResult> transaction::Methods::countAsync(
std::string const& collectionName, transaction::CountType type,
OperationOptions const& options) {
return countInternal(collectionName, type, options, Api::Asynchronous);
return countInternal(collectionName, type, options, MethodsApi::Asynchronous);
}

#ifndef USE_ENTERPRISE
Expand Down Expand Up @@ -2468,7 +2468,7 @@ Future<Result> Methods::replicateOperations(
return futures::collectAll(std::move(futures)).thenValue(std::move(cb));
}

Future<Result> Methods::commitInternal(Api api) {
Future<Result> Methods::commitInternal(MethodsApi api) {
TRI_IF_FAILURE("TransactionCommitFail") { return Result(TRI_ERROR_DEBUG); }

if (_state == nullptr || _state->status() != transaction::Status::RUNNING) {
Expand Down Expand Up @@ -2511,7 +2511,7 @@ Future<Result> Methods::commitInternal(Api api) {
});
}

Future<Result> Methods::abortInternal(Api api) {
Future<Result> Methods::abortInternal(MethodsApi api) {
if (_state == nullptr || _state->status() != transaction::Status::RUNNING) {
// transaction not created or not running
return Result(TRI_ERROR_TRANSACTION_INTERNAL,
Expand Down Expand Up @@ -2543,7 +2543,7 @@ Future<Result> Methods::abortInternal(Api api) {
});
}

Future<Result> Methods::finishInternal(Result const& res, Api api) {
Future<Result> Methods::finishInternal(Result const& res, MethodsApi api) {
if (res.ok()) {
// there was no previous error, so we'll commit
return this->commitInternal(api);
Expand All @@ -2556,7 +2556,7 @@ Future<Result> Methods::finishInternal(Result const& res, Api api) {
}

Future<OperationResult> Methods::documentInternal(std::string const& cname, VPackSlice value,
OperationOptions const& options, Api api) {
OperationOptions const& options, MethodsApi api) {
TRI_ASSERT(_state->status() == transaction::Status::RUNNING);

if (!value.isObject() && !value.isArray()) {
Expand All @@ -2578,7 +2578,7 @@ Future<OperationResult> Methods::documentInternal(std::string const& cname, VPac
}

Future<OperationResult> Methods::insertInternal(std::string const& cname, VPackSlice value,
OperationOptions const& options, Api api) {
OperationOptions const& options, MethodsApi api) {
TRI_ASSERT(_state->status() == transaction::Status::RUNNING);

if (!value.isObject() && !value.isArray()) {
Expand Down Expand Up @@ -2609,7 +2609,7 @@ Future<OperationResult> Methods::insertInternal(std::string const& cname, VPackS
}

Future<OperationResult> Methods::updateInternal(std::string const& cname, VPackSlice newValue,
OperationOptions const& options, Api api) {
OperationOptions const& options, MethodsApi api) {
TRI_ASSERT(_state->status() == transaction::Status::RUNNING);

if (!newValue.isObject() && !newValue.isArray()) {
Expand Down Expand Up @@ -2638,7 +2638,7 @@ Future<OperationResult> Methods::updateInternal(std::string const& cname, VPackS
}

Future<OperationResult> Methods::replaceInternal(std::string const& cname, VPackSlice newValue,
OperationOptions const& options, Api api) {
OperationOptions const& options, MethodsApi api) {
TRI_ASSERT(_state->status() == transaction::Status::RUNNING);

if (!newValue.isObject() && !newValue.isArray()) {
Expand Down Expand Up @@ -2667,7 +2667,7 @@ Future<OperationResult> Methods::replaceInternal(std::string const& cname, VPack
}

Future<OperationResult> Methods::removeInternal(std::string const& cname, VPackSlice value,
OperationOptions const& options, Api api) {
OperationOptions const& options, MethodsApi api) {
TRI_ASSERT(_state->status() == transaction::Status::RUNNING);

if (!value.isObject() && !value.isArray() && !value.isString()) {
Expand Down Expand Up @@ -2696,7 +2696,7 @@ Future<OperationResult> Methods::removeInternal(std::string const& cname, VPackS
}

Future<OperationResult> Methods::truncateInternal(std::string const& collectionName,
OperationOptions const& options, Api api) {
OperationOptions const& options, MethodsApi api) {
TRI_ASSERT(_state->status() == transaction::Status::RUNNING);

OperationOptions optionsCopy = options;
Expand All @@ -2714,7 +2714,7 @@ Future<OperationResult> Methods::truncateInternal(std::string const& collectionN
futures::Future<OperationResult> Methods::countInternal(std::string const& collectionName,
CountType type,
OperationOptions const& options,
Api api) {
MethodsApi api) {
TRI_ASSERT(_state->status() == transaction::Status::RUNNING);

if (_state->isCoordinator()) {
Expand Down
27 changes: 11 additions & 16 deletions arangod/Transaction/Methods.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "Rest/CommonDefines.h"
#include "Transaction/CountCache.h"
#include "Transaction/Hints.h"
#include "Transaction/MethodsApi.h"
#include "Transaction/Options.h"
#include "Transaction/Status.h"
#include "Utils/OperationResult.h"
Expand Down Expand Up @@ -87,12 +88,6 @@ class TransactionCollection;
namespace transaction {

class Methods {
protected:
enum class Api {
Asynchronous,
Synchronous
};

public:
template<typename T>
using Future = futures::Future<T>;
Expand Down Expand Up @@ -449,35 +444,35 @@ class Methods {
// APIs via an additional parameter, so `skipScheduler` can be set for network
// requests.
// TODO set skipScheduler: true for network requests
auto commitInternal(Api api) -> Future<Result>;
auto commitInternal(MethodsApi api) -> Future<Result>;
// TODO set skipScheduler: true for network requests
auto abortInternal(Api api) -> Future<Result>;
auto abortInternal(MethodsApi api) -> Future<Result>;
// TODO set skipScheduler: true for network requests
auto finishInternal(Result const& res, Api api) -> Future<Result>;
auto finishInternal(Result const& res, MethodsApi api) -> Future<Result>;
// is virtual for IgnoreNoAccessMethods
// TODO set skipScheduler: true for network requests
ENTERPRISE_VIRT auto documentInternal(std::string const& cname, VPackSlice value,
OperationOptions const& options, Api api)
OperationOptions const& options, MethodsApi api)
-> Future<OperationResult>;
// TODO set skipScheduler: true for network requests
auto insertInternal(std::string const& collectionName, VPackSlice value,
OperationOptions const& options, Api api) -> Future<OperationResult>;
OperationOptions const& options, MethodsApi api) -> Future<OperationResult>;
// TODO set skipScheduler: true for network requests
auto updateInternal(std::string const& collectionName, VPackSlice updateValue,
OperationOptions const& options, Api api) -> Future<OperationResult>;
OperationOptions const& options, MethodsApi api) -> Future<OperationResult>;
// TODO set skipScheduler: true for network requests
auto replaceInternal(std::string const& collectionName, VPackSlice replaceValue,
OperationOptions const& options, Api api) -> Future<OperationResult>;
OperationOptions const& options, MethodsApi api) -> Future<OperationResult>;
// TODO set skipScheduler: true for network requests
auto removeInternal(std::string const& collectionName, VPackSlice value,
OperationOptions const& options, Api api) -> Future<OperationResult>;
OperationOptions const& options, MethodsApi api) -> Future<OperationResult>;
// TODO set skipScheduler: true for network requests
auto truncateInternal(std::string const& collectionName, OperationOptions const& options,
Api api) -> Future<OperationResult>;
MethodsApi api) -> Future<OperationResult>;
// is virtual for IgnoreNoAccessMethods
// TODO set skipScheduler: true for network requests
ENTERPRISE_VIRT auto countInternal(std::string const& collectionName, CountType type,
OperationOptions const& options, Api api)
OperationOptions const& options, MethodsApi api)
-> futures::Future<OperationResult>;

/// @brief return the transaction collection for a document collection
Expand Down
30 changes: 30 additions & 0 deletions arangod/Transaction/MethodsApi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
////////////////////////////////////////////////////////////////////////////////
/// DISCLAIMER
///
/// Copyright 2021-2021 ArangoDB GmbH, Cologne, Germany
///
/// Licensed under the Apache License, Version 2.0 (the "License");
/// you may not use this file except in compliance with the License.
/// You may obtain a copy of the License at
///
/// http://www.apache.org/licenses/LICENSE-2.0
///
/// Unless required by applicable law or agreed to in writing, software
/// distributed under the License is distributed on an "AS IS" BASIS,
/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
/// See the License for the specific language governing permissions and
/// limitations under the License.
///
/// Copyright holder is ArangoDB GmbH, Cologne, Germany
///
/// @author Tobias Gödderz
////////////////////////////////////////////////////////////////////////////////

#pragma once

namespace arangodb::transaction {
enum class MethodsApi {
Asynchronous,
Synchronous
};
}
0