8000 Allow database server start with invalid repl factor settings (#17884) · cloudhub-js/arangodb@2a7fae6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2a7fae6

Browse files
jsteemannKVS85
jsteemann
and
KVS85
authored
Allow database server start with invalid repl factor settings (arangodb#17884)
* Allow database server start with invalid repl factor settings * Allow database server start in cluster even in case there are existing databases that would violate the settings `--cluster.min-replication-factor` or `--cluster.max-replication-factor`. This allows upgrading from older versions in which the replication factor validation for databases was not yet present. * Update CHANGELOG Co-authored-by: Vadim <vadim@arangodb.com>
1 parent b704fcb commit 2a7fae6

File tree

6 files changed

+85
-56
lines changed

6 files changed

+85
-56
lines changed

‎CHANGELOG

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
v3.9.7 (XXXX-XX-XX)
22
-------------------
33

4+
* Allow cluster database servers to start even when there are existing databases
5+
that would violate the settings `--cluster.min-replication-factor` or
6+
`--cluster.max-replication-factor`.
7+
This allows upgrading from older versions in which the replication factor
8+
validation for databases was not yet present.
9+
410
* Make the cache_oblivious option of jemalloc configurable from the environment.
511
This helps to save 4096 bytes of RAM for every allocation which is at least
612
16384 bytes large. This is particularly beneficial for the RocksDB buffer

‎arangod/Cluster/HeartbeatThread.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1372,13 +1372,18 @@ bool HeartbeatThread::handlePlanChangeCoordinator(uint64_t currentPlanVersion) {
13721372
continue;
13731373
}
13741374

1375-
arangodb::CreateDatabaseInfo info(_server, ExecContext::current());
13761375
TRI_ASSERT(options.value.get("name").isString());
1376+
CreateDatabaseInfo info(_server, ExecContext::current());
1377+
// set strict validation for database options to false.
1378+
// we don't want the heartbeat thread to fail here in case some
1379+
// invalid settings are present
1380+
info.strictValidation(false);
13771381
// when loading we allow system database names
13781382
auto infoResult = info.load(options.value, VPackSlice::emptyArraySlice());
13791383
if (infoResult.fail()) {
13801384
LOG_TOPIC("3fa12", ERR, Logger::HEARTBEAT)
1381-
<< "In agency database plan" << infoResult.errorMessage();
1385+
<< "in agency database plan for database " << options.value.toJson()
1386+
<< ": " << infoResult.errorMessage();
13821387
TRI_ASSERT(false);
13831388
}
13841389

‎arangod/RestHandler/RestAdminServerHandler.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,8 @@ void RestAdminServerHandler::handleMode() {
251251
}
252252

253253
void RestAdminServerHandler::handleDatabaseDefaults() {
254-
auto defaults = getVocbaseOptions(server(), VPackSlice::emptyObjectSlice());
254+
auto defaults = getVocbaseOptions(server(), VPackSlice::emptyObjectSlice(),
255+
/*strictValidation*/ false);
255256
VPackBuilder builder;
256257

257258
builder.openObject();

‎arangod/RestServer/DatabaseFeature.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,13 +1531,19 @@ ErrorCode DatabaseFeature::iterateDatabases(VPackSlice const& databases) {
15311531
// open the database and scan collections in it
15321532

15331533
// try to open this database
1534-
arangodb::CreateDatabaseInfo info(server(), ExecContext::current());
1534+
CreateDatabaseInfo info(server(), ExecContext::current());
1535+
// set strict validation for database options to false.
1536+
// we don't want the server start to fail here in case some
1537+
// invalid settings are present
1538+
info.strictValidation(false);
15351539
auto res = info.load(it, VPackSlice::emptyArraySlice());
1540+
15361541
if (res.fail()) {
1542+
std::string errorMsg;
15371543
if (res.is(TRI_ERROR_ARANGO_DATABASE_NAME_INVALID)) {
1544+
errorMsg.append(res.errorMessage());
15381545
// special case: if we find an invalid database name during startup,
15391546
// we will give the user some hint how to fix it
1540-
std::string errorMsg(res.errorMessage());
15411547
errorMsg.append(": '").append(databaseName).append("'");
15421548
// check if the name would be allowed when using extended names
15431549
if (DatabaseNameValidator::isAllowedName(
@@ -1549,9 +1555,14 @@ ErrorCode DatabaseFeature::iterateDatabases(VPackSlice const& databases) {
15491555
"be enabled via the startup option "
15501556
"`--database.extended-names-databases true`");
15511557
}
1552-
res.reset(TRI_ERROR_ARANGO_DATABASE_NAME_INVALID,
1553-
std::move(errorMsg));
1558+
} else {
1559+
errorMsg.append("when opening database '")
1560+
.append(databaseName)
1561+
.append("': ");
1562+
errorMsg.append(res.errorMessage());
15541563
}
1564+
1565+
res.reset(res.errorNumber(), std::move(errorMsg));
15551566
THROW_ARANGO_EXCEPTION(res);
15561567
}
15571568

‎arangod/VocBase/VocbaseInfo.cpp

Lines changed: 44 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,9 @@ Result CreateDatabaseInfo::load(std::string const& name, uint64_t id) {
6464

6565
Result CreateDatabaseInfo::load(VPackSlice options, VPackSlice users) {
6666
Result res = extractOptions(options, true /*getId*/, true /*getUser*/);
67-
if (!res.ok()) {
68-
return res;
67+
if (res.ok()) {
68+
res = extractUsers(users);
6969
}
70-
71-
res = extractUsers(users);
7270
if (!res.ok()) {
7371
return res;
7472
}
@@ -85,11 +83,9 @@ Result CreateDatabaseInfo::load(std::string const& name, VPackSlice options,
8583
_name = methods::Databases::normalizeName(name);
8684

8785
Result res = extractOptions(options, true /*getId*/, false /*getName*/);
88-
if (!res.ok()) {
89-
return res;
86+
if (res.ok()) {
87+
res = extractUsers(users);
9088
}
91-
92-
res = extractUsers(users);
9389
if (!res.ok()) {
9490
return res;
9591
}
@@ -107,11 +103,9 @@ Result CreateDatabaseInfo::load(std::string const& name, uint64_t id,
107103
_id = id;
108104

109105
Result res = extractOptions(options, false /*getId*/, false /*getUser*/);
110-
if (!res.ok()) {
111-
return res;
106+
if (res.ok()) {
107+
res = extractUsers(users);
112108
}
113-
114-
res = extractUsers(users);
115109
if (!res.ok()) {
116110
return res;
117111
}
@@ -236,37 +230,45 @@ Result CreateDatabaseInfo::extractOptions(VPackSlice options, bool extractId,
236230
return Result(TRI_ERROR_HTTP_BAD_PARAMETER, "invalid options slice");
237231
}
238232

239-
auto vocopts = getVocbaseOptions(_server, options);
240-
_replicationFactor = vocopts.replicationFactor;
241-
_writeConcern = vocopts.writeConcern;
242-
_sharding = vocopts.sharding;
233+
try {
234+
auto vocopts = getVocbaseOptions(_server, options, _strictValidation);
235+
_replicationFactor = vocopts.replicationFactor;
236+
_writeConcern = vocopts.writeConcern;
237+
_sharding = vocopts.sharding;
243238

244-
if (extractName) {
245-
auto nameSlice = options.get(StaticStrings::DatabaseName);
246-
if (!nameSlice.isString()) {
247-
return Result(TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD, "no valid id given");
239+
if (extractName) {
240+
auto nameSlice = options.get(StaticStrings::DatabaseName);
241+
if (!nameSlice.isString()) {
242+
return Result(TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD, "no valid id given");
243+
}
244+
_name = methods::Databases::normalizeName(nameSlice.copyString());
248245
}
249-
_name = methods::Databases::normalizeName(nameSlice.copyString());
250-
}
251-
if (extractId) {
252-
auto idSlice = options.get(StaticStrings::DatabaseId);
253-
if (idSlice.isString()) {
254-
// improve once it works
255-
// look for some nice internal helper this has proably been done before
256-
auto idStr = idSlice.copyString();
257-
_id = basics::StringUtils::uint64(idSlice.stringRef().data(),
258-
idSlice.stringRef().size());
259-
260-
} else if (idSlice.isUInt()) {
261-
_id = idSlice.getUInt();
262-
} else if (idSlice.isNone()) {
263-
// do nothing - can be set later - this sucks
264-
} else {
265-
return Result(TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD, "no valid id given");
246+
if (extractId) {
247+
auto idSlice = options.get(StaticStrings::DatabaseId);
248+
if (idSlice.isString()) {
249+
// improve once it works
250+
// look for some nice internal helper this has proably been done before
251+
auto idStr = idSlice.copyString();
252+
_id = basics::StringUtils::uint64(idSlice.stringView().data(),
253+
idSlice.stringView().size());
254+
255+
} else if (idSlice.isUInt()) {
256+
_id = idSlice.getUInt();
257+
} else if (idSlice.isNone()) {
258+
// do nothing - can be set later - this sucks
259+
} else {
260+
return Result(TRI_ERROR_ARANGO_DOCUMENT_KEY_BAD, "no valid id given");
261+
}
266262
}
267-
}
268263

269-
return checkOptions();
264+
return checkOptions();
265+
} catch (basics::Exception const& ex) {
266+
return Result(ex.code(), ex.what());
267+
} catch (std::exception const& ex) {
268+
return Result(TRI_ERROR_INTERNAL, ex.what());
269+
} catch (...) {
270+
return Result(TRI_ERROR_INTERNAL, "an unknown error occurred");
271+
}
270272
}
271273

272274
Result CreateDatabaseInfo::checkOptions() {
@@ -286,8 +288,8 @@ Result CreateDatabaseInfo::checkOptions() {
286288
}
287289

288290
VocbaseOptions getVocbaseOptions(
289-
application_features::ApplicationServer& server,
290-
VPackSlice const& options) {
291+
application_features::ApplicationServer& server, VPackSlice options,
292+
bool strictValidation) {
291293
TRI_ASSERT(options.isObject());
292294
// Invalid options will be silently ignored. Default values will be used
293295
// instead.
@@ -334,7 +336,7 @@ VocbaseOptions getVocbaseOptions(
334336
vocbaseOptions.replicationFactor =
335337
replicationSlice
336338
.getNumber<decltype(vocbaseOptions.replicationFactor)>();
337-
if (haveCluster) {
339+
if (haveCluster && strictValidation) {
338340
uint32_t const minReplicationFactor =
339341
server.getFeature<ClusterFeature>().minReplicationFactor();
340342
uint32_t const maxReplicationFactor =

‎arangod/VocBase/VocbaseInfo.h

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,13 @@ class CreateDatabaseInfo {
9999
return _id;
100100
}
101101

102-
bool valid() const { return _valid; }
102+
void strictValidation(bool value) noexcept { _strictValidation = value; }
103103

104-
bool validId() const { return _validId; }
104+
bool valid() const noexcept { return _valid; }
105105

106-
// shold be created with vaild id
106+
bool validId() const noexcept { return _validId; }
107+
108+
// shold be created with valid id
107109
void setId(uint64_t id) {
108110
_id = id;
109111
_validId = true;
@@ -123,10 +125,12 @@ class CreateDatabaseInfo {
123125
TRI_ASSERT(_valid);
124126
return _writeConcern;
125127
}
128+
126129
std::string const& sharding() const {
127130
TRI_ASSERT(_valid);
128131
return _sharding;
129132
}
133+
130134
void sharding(std::string const& sharding) { _sharding = sharding; }
131135

132136
ShardingPrototype shardingPrototype() const;
@@ -151,19 +155,19 @@ class CreateDatabaseInfo {
151155
std::uint32_t _writeConcern = 1;
152156
ShardingPrototype _shardingPrototype = ShardingPrototype::Undefined;
153157

158+
bool _strictValidation = true;
154159
bool _validId = false;
155-
bool _valid =
156-
false; // required because TRI_ASSERT needs variable in Release mode.
160+
bool _valid = false;
157161
};
158162

159163
struct VocbaseOptions {
160-
std::string sharding = "";
164+
std::string sharding;
161165
std::uint32_t replicationFactor = 1;
162166
std::uint32_t writeConcern = 1;
163167
};
164168

165169
VocbaseOptions getVocbaseOptions(application_features::ApplicationServer&,
166-
velocypack::Slice const&);
170+
velocypack::Slice, bool strictValidation);
167171

168172
void addClusterOptions(VPackBuilder& builder, std::string const& sharding,
169173
std::uint32_t replicationFactor,

0 commit comments

Comments
 (0)
0