8000 Cleanup in preparation for Replication 2.0 transaction handling (try … · arangodb/arangodb@334c02a · GitHub
[go: up one dir, main page]
Skip to content

Commit 334c02a

Browse files
goedderzmpoeter
goedderz
and
mpoeter
authored
Cleanup in preparation for Replication 2.0 transaction handling (try #2) (#14907)
* Took over some refactoring from a previous branches * Took over some more changes * Fixes * Fixed catchToResultT * Re-added missing function * Moved default initializations * Fixed C++ tests * Improved the assertion on single server a little * Suppressed warning * Fixed remaining merge conflicts * Added comments, assertions, and moved initializations in transaction::Options from member initializer list to default member initializers * Fixed compile error * [WIP] Try to fix google test error * Fix test * Fix non-maintainer build. Co-authored-by: mpoeter <manuel@manuel-poeter.at>
1 parent 258febe commit 334c02a

26 files changed

+322
-218
lines changed

‎arangod/Cluster/ClusterTrxMethods.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ Future<Result> commitAbortTransaction(arangodb::TransactionState* state,
271271
Result res;
272272
for (Try<arangodb::network::Response> const& tryRes : responses) {
273273
network::Response const& resp = tryRes.get(); // throws exceptions upwards
274-
Result res = ::checkTransactionResult(tidPlus, status, resp);
274+
res = ::checkTransactionResult(tidPlus, status, resp);
275275
if (res.fail()) {
276276
break;
277277
}
@@ -345,8 +345,7 @@ Future<Result> commitAbortTransaction(transaction::Methods& trx, transaction::St
345345

346346
} // namespace
347347

348-
namespace arangodb {
349-
namespace ClusterTrxMethods {
348+
namespace arangodb::ClusterTrxMethods {
350349
using namespace arangodb::futures;
351350

352351
bool IsServerIdLessThan::operator()(ServerID const& lhs, ServerID const& rhs) const noexcept {
@@ -575,5 +574,4 @@ bool isElCheapo(TransactionState const& state) {
575574
state.hasHint(transaction::Hints::Hint::FROM_TOPLEVEL_AQL));
576575
}
577576

578-
} // namespace ClusterTrxMethods
579-
} // namespace arangodb
577+
} // namespace arangodb::ClusterTrxMethods

‎arangod/Cluster/ClusterTypes.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,4 +182,10 @@ AnalyzersRevision::Ptr AnalyzersRevision::fromVelocyPack(VPackSlice const& slice
182182
buildingRevisionSlice.getNumber<AnalyzersRevision::Revision>(),
183183
std::move(coordinatorID), rebootID));
184184
}
185+
186+
auto isShardName(std::string_view name) -> bool {
187+
return name.size() > 1 && name[0] == 's' &&
188+
std::all_of(name.cbegin() + 1, name.cend(), static_cast<int(&)(int)>(std::isdigit));
189+
}
190+
185191
} // namespace arangodb

‎arangod/Cluster/ServerState.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,3 +1190,11 @@ Result ServerState::propagateClusterReadOnly(bool mode) {
11901190
setReadOnly(mode ? API_TRUE : API_FALSE);
11911191
return Result();
11921192
}
1193+
1194+
#ifdef ARANGODB_USE_GOOGLE_TESTS
1195+
bool ServerState::isGoogleTest() const noexcept { return _isGoogleTests; }
1196+
1197+
void ServerState::setGoogleTest(bool isGoogleTests) noexcept {
1198+
_isGoogleTests = isGoogleTests;
1199+
}
1200+
#endif

‎arangod/Cluster/ServerState.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,13 @@ class ServerState {
282282
/// file where the server persists its UUID
283283
std::string getUuidFilename() const;
284284

285+
#ifdef ARANGODB_USE_GOOGLE_TESTS
286+
[[nodiscard]] bool isGoogleTest() const noexcept;
287+
void setGoogleTest(bool isGoogleTests) noexcept;
288+
#else
289+
[[nodiscard]] constexpr bool isGoogleTest() const noexcept { return false; }
290+
#endif
291+
285292
private:
286293
/// @brief atomically fetches the server role
287294
inline RoleEnum loadRole() const noexcept {
@@ -374,6 +381,10 @@ class ServerState {
374381
TRI_voc_tick_t _foxxmasterSince;
375382

376383
bool _foxxmasterQueueupdate;
384+
385+
#ifdef ARANGODB_USE_GOOGLE_TESTS
386+
bool _isGoogleTests = false;
387+
#endif
377388
};
378389
} // namespace arangodb
379390

‎arangod/ClusterEngine/ClusterTransactionState.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,10 @@ uint64_t ClusterTransactionState::numCommits() const {
155155
return _status == transaction::Status::COMMITTED ? 1 : 0;
156156
}
157157

158+
TRI_voc_tick_t ClusterTransactionState::lastOperationTick() const noexcept {
159+
return 0;
160+
}
161+
158162
std::unique_ptr<TransactionCollection> ClusterTransactionState::createTransactionCollection(
159163
DataSourceId cid, AccessMode::Type accessType) {
160164
return std::make_unique<ClusterTransactionCollection>(this, cid, accessType);

‎arangod/ClusterEngine/ClusterTransactionState.h

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,34 +24,43 @@
2424
#pragma once
2525

2626
#include "StorageEngine/TransactionState.h"
27+
#include "VocBase/Identifiers/TransactionId.h"
28+
29+
struct TRI_vocbase_t;
2730

2831
namespace arangodb {
32+
class Result;
33+
34+
namespace transaction {
35+
struct Options;
36+
}
2937

3038
/// @brief transaction type
3139
class ClusterTransactionState final : public TransactionState {
3240
public:
3341
ClusterTransactionState(TRI_vocbase_t& vocbase, TransactionId tid,
3442
transaction::Options const& options);
35-
~ClusterTransactionState() = default;
43+
~ClusterTransactionState() override = default;
3644

3745
/// @brief begin a transaction
38-
Result beginTransaction(transaction::Hints hints) override;
46+
[[nodiscard]] Result beginTransaction(transaction::Hints hints) override;
3947

4048
/// @brief commit a transaction
41-
Result commitTransaction(transaction::Methods* trx) override;
49+
[[nodiscard]] Result commitTransaction(transaction::Methods* trx) override;
4250

4351
/// @brief abort a transaction
44-
Result abortTransaction(transaction::Methods* trx) override;
45-
52+
[[nodiscard]] Result abortTransaction(transaction::Methods* trx) override;
53+
4654
/// @brief return number of commits, including intermediate commits
47-
uint64_t numCommits() const override;
55+
[[nodiscard]] uint64_t numCommits() const override;
4856

49-
bool hasFailedOperations() const override { return false; }
57+
[[nodiscard]] bool hasFailedOperations() const override { return false; }
58+
59+
[[nodiscard]] TRI_voc_tick_t lastOperationTick() const noexcept override;
5060

5161
protected:
5262
std::unique_ptr<TransactionCollection> createTransactionCollection(
5363
DataSourceId cid, AccessMode::Type accessType) override;
5464
};
5565

5666
} // namespace arangodb
57-

‎arangod/IResearch/IResearchAqlAnalyzer.cpp

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -185,36 +185,41 @@ bool normalize_slice(VPackSlice const& slice, VPackBuilder& builder) {
185185
class CalculationTransactionState final : public arangodb::TransactionState {
186186
public:
187187
explicit CalculationTransactionState(TRI_vocbase_t& vocbase)
188-
: TransactionState(vocbase, arangodb::TransactionId(0), arangodb::transaction::Options()) {
188+
: TransactionState(vocbase, arangodb::TransactionId(0),
189+
arangodb::transaction::Options()) {
189190
updateStatus(arangodb::transaction::Status::RUNNING); // always running to make ASSERTS happy
190191
}
191192

192-
~CalculationTransactionState() {
193+
~CalculationTransactionState() override {
193194
if (status() == arangodb::transaction::Status::RUNNING) {
194195
updateStatus(arangodb::transaction::Status::ABORTED); // simulate state changes to make ASSERTS happy
195196
}
196197
}
197198
/// @brief begin a transaction
198-
arangodb::Result beginTransaction(arangodb::transaction::Hints) override {
199+
[[nodiscard]] arangodb::Result beginTransaction(arangodb::transaction::Hints) override {
199200
return {};
200201
}
201202

202203
/// @brief commit a transaction
203-
arangodb::Result commitTransaction(arangodb::transaction::Methods*) override {
204+
[[nodiscard]] arangodb::Result commitTransaction(arangodb::transaction::Methods*) override {
204205
updateStatus(arangodb::transaction::Status::COMMITTED); // simulate state changes to make ASSERTS happy
205206
return {};
206207
}
207208

208209
/// @brief abort a transaction
209-
arangodb::Result abortTransaction(arangodb::transaction::Methods*) override {
210+
[[nodiscard]] arangodb::Result abortTransaction(arangodb::transaction::Methods*) override {
210211
updateStatus(arangodb::transaction::Status::ABORTED); // simulate state changes to make ASSERTS happy
211212
return {};
212213
}
213214

214-
bool hasFailedOperations() const override { return false; }
215+
[[nodiscard]] bool hasFailedOperations() const override { return false; }
215216

216217
/// @brief number of commits, including intermediate commits
217-
uint64_t numCommits() const override { return 0; }
218+
[[nodiscard]] uint64_t numCommits() const override { return 0; }
219+
220+
[[nodiscard]] TRI_voc_tick_t lastOperationTick() const noexcept override {
221+
return 0;
222+
}
218223

219224
std::unique_ptr<arangodb::TransactionCollection> createTransactionCollection(
220225
arangodb::DataSourceId cid, arangodb::AccessMode::Type accessType) override {

‎arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@
2929
#include <rocksdb/status.h>
3030

3131
using namespace arangodb;
32-
33-
RocksDBReadOnlyBaseMethods::RocksDBReadOnlyBaseMethods(RocksDBTransactionState* state)
34-
: RocksDBTransactionMethods(state) {}
3532

3633
void RocksDBReadOnlyBaseMethods::prepareOperation(DataSourceId cid, RevisionId rid,
3734
TRI_voc_document_operation_e operationType) {

‎arangod/RocksDBEngine/Methods/RocksDBReadOnlyBaseMethods.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ namespace arangodb {
3333

3434
class RocksDBReadOnlyBaseMethods : public RocksDBTransactionMethods {
3535
public:
36-
explicit RocksDBReadOnlyBaseMethods(RocksDBTransactionState* state);
36+
using RocksDBTransactionMethods::RocksDBTransactionMethods;
3737

3838
TRI_voc_tick_t lastOperationTick() const noexcept override { return 0; }
3939

‎arangod/RocksDBEngine/RocksDBTransactionState.cpp

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,8 @@ Result RocksDBTransactionState::commitTransaction(transaction::Methods* activeTr
215215
cleanupTransaction(); // deletes trx
216216
++statistics()._transactionsCommitted;
217217
} else {
218-
abortTransaction(activeTrx); // deletes trx
218+
// what if this fails?
219+
std::ignore = abortTransaction(activeTrx); // deletes trx
219220
}
220221
TRI_ASSERT(!_cacheTx);
221222

@@ -330,18 +331,36 @@ void RocksDBTransactionState::trackIndexRemove(DataSourceId cid, IndexId idxId,
330331
}
331332
}
332333

333-
bool RocksDBTransactionState::isOnlyExclusiveTransaction() const {
334+
bool RocksDBTransactionState::isOnlyExclusiveTransaction() const noexcept {
334335
if (!AccessMode::isWriteOrExclusive(_type)) {
335336
return false;
336337
}
337-
for (TransactionCollection* coll : _collections) {
338-
if (AccessMode::isWrite(coll->accessType())) {
339-
return false;
340-
}
341-
}
342-
return true;
338+
return std::none_of(_collections.cbegin(), _collections.cend(), [](auto* coll) {
339+
return AccessMode::isWrite(coll->accessType());
340+
});
343341
}
344342

343+
bool RocksDBTransactionState::hasFailedOperations() const {
344+
return (_status == transaction::Status::ABORTED) && hasOperations();
345+
}
346+
347+
RocksDBTransactionState* RocksDBTransactionState::toState(transaction::Methods* trx) {
348+
TRI_ASSERT(trx != nullptr);
349+
TransactionState* state = trx->state();
350+
TRI_ASSERT(state != nullptr);
351+
return static_cast<RocksDBTransactionState*>(state);
352+
}
353+
354+
RocksDBTransactionMethods* RocksDBTransactionState::toMethods(transaction::Methods* trx, DataSourceId collectionId) {
355+
TRI_ASSERT(trx != nullptr);
356+
TransactionState* state = trx->state();
357+
TRI_ASSERT(state != nullptr);
358+
return static_cast<RocksDBTransactionState*>(state)->rocksdbMethods(collectionId);
359+
}
360+
361+
void RocksDBTransactionState::prepareForParallelReads() { _parallel = true; }
362+
bool RocksDBTransactionState::inParallelMode() const { return _parallel; }
363+
345364
#ifdef ARANGODB_ENABLE_MAINTAINER_MODE
346365
RocksDBTransactionStateGuard::RocksDBTransactionStateGuard(RocksDBTransactionState* state) noexcept
347366
: _state(state) {

‎arangod/RocksDBEngine/RocksDBTransactionState.h

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -66,65 +66,53 @@ class RocksDBTransactionState : public TransactionState {
6666
public:
6767
RocksDBTransactionState(TRI_vocbase_t& vocbase, TransactionId tid,
6868
transaction::Options const& options);
69-
~RocksDBTransactionState();
69+
~RocksDBTransactionState() override;
7070

7171
/// @brief begin a transaction
72-
Result beginTransaction(transaction::Hints hints) override;
72+
[[nodiscard]] Result beginTransaction(transaction::Hints hints) override;
7373

7474
/// @brief commit a transaction
75-
Result commitTransaction(transaction::Methods* trx) override;
75+
[[nodiscard]] Result commitTransaction(transaction::Methods* trx) override;
7676

7777
/// @brief abort a transaction
78-
Result abortTransaction(transaction::Methods* trx) override;
78+
[[nodiscard]] Result abortTransaction(transaction::Methods* trx) override;
7979

80-
virtual bool hasOperations() const noexcept = 0;
80+
[[nodiscard]] virtual bool hasOperations() const noexcept = 0;
8181

82-
virtual uint64_t numOperations() const noexcept = 0;
82+
[[nodiscard]] virtual uint64_t numOperations() const noexcept = 0;
8383

84-
bool hasFailedOperations() const override {
85-
return (_status == transaction::Status::ABORTED) && hasOperations();
86-
}
84+
[[nodiscard]] bool hasFailedOperations() const override;
8785

88-
bool iteratorMustCheckBounds(DataSourceId cid, ReadOwnWrites readOwnWrites) const;
86+
[[nodiscard]] bool iteratorMustCheckBounds(DataSourceId cid, ReadOwnWrites readOwnWrites) const;
8987

9088
void prepareOperation(DataSourceId cid, RevisionId rid,
9189
TRI_voc_document_operation_e operationType);
9290

9391
/// @brief add an operation for a transaction collection
9492
/// sets hasPerformedIntermediateCommit to true if an intermediate commit was
9593
/// performed
96-
Result addOperation(DataSourceId collectionId, RevisionId revisionId,
94+
[[nodiscard]] Result addOperation(DataSourceId collectionId, RevisionId revisionId,
9795
TRI_voc_document_operation_e opType,
9896
bool& hasPerformedIntermediateCommit);
9997

10098
/// @brief return wrapper around rocksdb transaction
101-
virtual RocksDBTransactionMethods* rocksdbMethods(DataSourceId collectionId) const = 0;
102-
99+
[[nodiscard]] virtual RocksDBTransactionMethods* rocksdbMethods(DataSourceId collectionId) const = 0;
100+
103101
/// @brief acquire a database snapshot if we do not yet have one.
104102
/// Returns true if a snapshot was acquired, otherwise false (i.e., if we already had a snapshot)
105-
virtual bool ensureSnapshot() = 0;
106-
107-
static RocksDBTransactionState* toState(transaction::Methods* trx) {
108-
TRI_ASSERT(trx != nullptr);
109-
TransactionState* state = trx->state();
110-
TRI_ASSERT(state != nullptr);
111-
return static_cast<RocksDBTransactionState*>(state);
112-
}
113-
114-
static RocksDBTransactionMethods* toMethods(transaction::Methods* trx, DataSourceId collectionId) {
115-
TRI_ASSERT(trx != nullptr);
116-
TransactionState* state = trx->state();
117-
TRI_ASSERT(state != nullptr);
118-
return static_cast<RocksDBTransactionState*>(state)->rocksdbMethods(collectionId);
119-
}
103+
[[nodiscard]] virtual bool ensureSnapshot() = 0;
104+
105+
[[nodiscard]] static RocksDBTransactionState* toState(transaction::Methods* trx);
106+
107+
[[nodiscard]] static RocksDBTransactionMethods* toMethods(transaction::Methods* trx, DataSourceId collectionId);
120108

121109
/// @brief make some internal preparations for accessing this state in
122110
/// parallel from multiple threads. READ-ONLY transactions
123-
void prepareForParallelReads() { _parallel = true; }
111+
void prepareForParallelReads();
124112
/// @brief in parallel mode. READ-ONLY transactions
125-
bool inParallelMode() const { return _parallel; }
113+
[[nodiscard]] bool inParallelMode() const;
126114

127-
RocksDBTransactionCollection::TrackedOperations& trackedOperations(DataSourceId cid);
115+
[[nodiscard]] RocksDBTransactionCollection::TrackedOperations& trackedOperations(DataSourceId cid);
128116

129117
/// @brief Track documents inserted to the collection
130118
/// Used to update the revision tree for replication after commit
@@ -142,9 +130,10 @@ class RocksDBTransactionState : public TransactionState {
142130
/// Used to update the estimate after the trx committed
143131
void trackIndexRemove(DataSourceId cid, IndexId idxObjectId, uint64_t hash);
144132

145-
bool isOnlyExclusiveTransaction() const;
133+
/// @brief whether or not a transaction only has exclusive or read accesses
134+
bool isOnlyExclusiveTransaction() const noexcept;
146135

147-
virtual rocksdb::SequenceNumber beginSeq() const = 0;
136+
[[nodiscard]] virtual rocksdb::SequenceNumber beginSeq() const = 0;
148137

149138
#ifdef ARANGODB_ENABLE_MAINTAINER_MODE
150139
/// @brief only needed for RocksDBTransactionStateGuard

‎arangod/StorageEngine/TransactionCollection.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class TransactionCollection {
6060

6161
std::string const& collectionName() const;
6262

63-
AccessMode::Type accessType() const { return _accessType; }
63+
AccessMode::Type accessType() const noexcept { return _accessType; }
6464

6565
Result updateUsage(AccessMode::Type accessType);
6666

0 commit comments

Comments
 (0)
0