8000 Merge branch 'devel' of github.com:arangodb/arangodb into feature/err… · arangodb/arangodb@7463f85 · GitHub
[go: up one dir, main page]
Skip to content

Commit 7463f85

Browse files
goedderzgoedderz
committed
Merge branch 'devel' of github.com:arangodb/arangodb into feature/error-messages-as-string-views
2 parents baf963f + 8f32165 commit 7463f85

18 files changed

+827
-84
lines changed

‎CHANGELOG

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
devel
22
-----
33

4+
* Fix error reporting in the reloadTLS route.
5+
46
* Fix potential undefined behavior when iterating over connected nodes in an
57
execution plan and calling callbacks for each of the nodes: if the callbacks
68
modified the list of connected nodes of the current that they were called

‎CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,6 +1246,7 @@ if (USE_MAINTAINER_MODE)
12461246
set(ERROR_FILES
12471247
lib/Basics/voc-errors.h
12481248
lib/Basics/voc-errors.cpp
1249+
lib/Basics/error-registry.h
12491250
js/common/bootstrap/errors.js
12501251
)
12511252

‎arangod/GeneralServer/GeneralServer.cpp

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040

4141
#include <chrono>
4242
#include <thread>
43+
#include <algorithm>
4344

4445
using namespace arangodb;
4546
using namespace arangodb::basics;
@@ -135,16 +136,16 @@ void GeneralServer::stopConnections() {
135136
}
136137

137138
void GeneralServer::stopWorking() {
138-
auto now = std::chrono::system_clock::now();
139+
auto const started {std::chrono::system_clock::now()};
140+
constexpr auto timeout {std::chrono::seconds(5)};
139141
do {
140-
std::unique_lock<std::recursive_mutex> guard(_tasksLock);
141-
bool done = _commTasks.empty();
142-
guard.unlock();
143-
if (done) {
144-
break;
142+
{
143+
std::unique_lock<std::recursive_mutex> guard(_tasksLock);
144+
if (_commTasks.empty())
145+
break;
145146
}
146147
std::this_thread::yield();
147-
} while((std::chrono::system_clock::now() - now) < std::chrono::seconds(5));
148+
} while((std::chrono::system_clock::now() - started) < timeout);
148149
{
149150
std::lock_guard<std::recursive_mutex> guard(_tasksLock);
150151
_commTasks.clear();
@@ -169,18 +170,8 @@ bool GeneralServer::openEndpoint(IoContext& ioContext, Endpoint* endpoint) {
169170
}
170171

171172
IoContext& GeneralServer::selectIoContext() {
172-
unsigned low = _contexts[0].clients();
173-
size_t lowpos = 0;
174-
175-
for (size_t i = 1; i < _contexts.size(); ++i) {
176-
unsigned x = _contexts[i].clients();
177-
if (x < low) {
178-
low = x;
179-
lowpos = i;
180-
}
181-
}
182-
183-
return _contexts[lowpos];
173+
return *std::min_element(_contexts.begin(), _contexts.end(),
174+
[](auto const &a, auto const &b) { return a.clients() < b.clients(); });
184175
}
185176

186177
#ifdef USE_ENTERPRISE

‎arangod/GeneralServer/GeneralServerFeature.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ Result GeneralServerFeature::reloadTLS() { // reload TLS data from disk
295295
Result res;
296296
for (auto& up : _servers) {
297297
Result res2 = up->reloadTLS();
298-
if (!res2.fail()) {
298+
if (res2.fail()) {
299299
res = res2; // yes, we only report the last error if there is one
300300
}
301301
}

‎arangod/Replication/DatabaseInitialSyncer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ arangodb::Result fetchRevisions(arangodb::transaction::Methods& trx,
154154
options.isRestore = true;
155155
options.validate = false; // no validation during replication
156156
options.indexOperationMode = arangodb::IndexOperationMode::internal;
157-
options.ignoreUniqueConstraints = true;
157+
options.checkUniqueConstraintsInPreflight = true;
158158
options.waitForSync = false; // no waitForSync during replication
159159
if (!state.leaderId.empty()) {
160160
options.isSynchronousReplicationFrom = state.leaderId;

‎arangod/RocksDBEngine/RocksDBCollection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,7 +1522,7 @@ Result RocksDBCollection::insertDocument(arangodb::transaction::Methods* trx,
15221522
RocksDBTransactionState* state = RocksDBTransactionState::toState(trx);
15231523
RocksDBMethods* mthds = state->rocksdbMethods();
15241524

1525-
if (options.ignoreUniqueConstraints) {
1525+
if (options.checkUniqueConstraintsInPreflight) {
15261526
// we can only afford the separation of checking and inserting in a
15271527
// transaction that disallows concurrency, i.e. we need to have the
15281528
// exclusive lock on the collection.
@@ -1673,7 +1673,7 @@ Result RocksDBCollection::updateDocument(transaction::Methods* trx,
16731673
RocksDBTransactionState* state = RocksDBTransactionState::toState(trx);
16741674
RocksDBMethods* mthds = state->rocksdbMethods();
16751675

1676-
if (options.ignoreUniqueConstraints) {
1676+
if (options.checkUniqueConstraintsInPreflight) {
16771677
// we can only afford the separation of checking and inserting in a
16781678
// transaction that disallows concurrency, i.e. we need to have the
16791679
// exclusive lock on the collection.

‎arangod/RocksDBEngine/RocksDBIncrementalSync.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ Result syncChunkRocksDB(DatabaseInitialSyncer& syncer, SingleCollectionTransacti
184184
options.indexOperationMode = IndexOperationMode::internal;
185185
options.waitForSync = false;
186186
options.validate = false;
187-
options.ignoreUniqueConstraints = true;
187+
options.checkUniqueConstraintsInPreflight = true;
188188

189189
if (!syncer._state.leaderId.empty()) {
190190
options.isSynchronousReplicationFrom = syncer._state.leaderId;

‎arangod/RocksDBEngine/RocksDBMethods.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
#ifndef ARANGOD_ROCKSDB_ROCKSDB_METHODS_H
2525
#define ARANGOD_ROCKSDB_ROCKSDB_METHODS_H 1
2626

27-
#include "Basics/Result.h"
2827
#include "RocksDBEngine/RocksDBCommon.h"
2928

3029
namespace rocksdb {
@@ -38,10 +37,6 @@ struct ReadOptions;
3837
} // namespace rocksdb
3938

4039
namespace arangodb {
41-
namespace transaction {
42-
class Methods;
43-
}
44-
4540
class RocksDBKey;
4641
class RocksDBMethods;
4742
class RocksDBTransactionState;

‎arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ Result RocksDBPrimaryIndex::insert(transaction::Methods& trx,
646646

647647
Result res;
648648

649-
if (!options.ignoreUniqueConstraints) {
649+
if (!options.checkUniqueConstraintsInPreflight) {
650650
res = probeKey(trx, mthd, key, keySlice, options, /*lock*/ true);
651651

652652
if (res.fail()) {

‎arangod/RocksDBEngine/RocksDBVPackIndex.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ Result RocksDBVPackIndex::insert(transaction::Methods& trx, RocksDBMethods* mthd
831831
transaction::StringLeaser leased(&trx);
832832
rocksdb::PinnableSlice existing(leased.get());
833833
for (RocksDBKey const& key : elements) {
834-
if (!options.ignoreUniqueConstraints) {
834+
if (!options.checkUniqueConstraintsInPreflight) {
835835
s = mthds->GetForUpdate(_cf, key.string(), &existing);
836836
if (s.ok()) { // detected conflicting index entry
837837
res.reset(TRI_ERROR_ARANGO_UNIQUE_CONSTRAINT_VIOLATED);

‎arangod/Utils/OperationOptions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ OperationOptions::OperationOptions()
4040
returnOld(false),
4141
returnNew(false),
4242
isRestore(false),
43-
ignoreUniqueConstraints(false),
43+
checkUniqueConstraintsInPreflight(false),
4444
truncateCompact(true),
4545
_context(nullptr) {}
4646

‎arangod/Utils/OperationOptions.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,14 @@ struct OperationOptions {
118118
// restored by replicated and arangorestore
119119
bool isRestore;
120120

121-
// for replication; only set true if you an guarantee that any conflicts have
122-
// already been removed, and are simply not reflected in the transaction read
123-
bool ignoreUniqueConstraints;
121+
// for replication; only set true if case insert/replace should have a read-only
122+
// preflight phase, in which it checks whether a document can actually be inserted
123+
// before carrying out the actual insert/replace.
124+
// separating the check phase from the actual insert/replace allows running the
125+
// preflight check without modifying the transaction's underlying WriteBatch object,
126+
// so in case a unique constraint violation is detected, it does not need to be
127+
// rebuilt (this would be _very_ expensive).
128+
bool checkUniqueConstraintsInPreflight;
124129

125130
// when truncating - should we also run the compaction?
126131
// defaults to true.

0 commit comments

Comments
 (0)
0