8000 a few usability fixes for minReplicationFactor/writeConcern (#10762) · nginxpre/arangodb@e9e2a74 · GitHub
[go: up one dir, main page]
Skip to content

Commit e9e2a74

Browse files
jsteemannjsteemann
authored
a few usability fixes for minReplicationFactor/writeConcern (arangodb#10762)
1 parent 40ca420 commit e9e2a74

File tree

4 files changed

+73
-10
lines changed

4 files changed

+73
-10
lines changed

‎arangod/Sharding/ShardingInfo.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,9 @@ ShardingInfo::ShardingInfo(arangodb::velocypack::Slice info, LogicalCollection*
7777
}
7878

7979
VPackSlice distributeShardsLike = info.get(StaticStrings::DistributeShardsLike);
80-
if (!distributeShardsLike.isNone() && !distributeShardsLike.isString()) {
80+
if (!distributeShardsLike.isNone() &&
81+
!distributeShardsLike.isString() &&
82+
!distributeShardsLike.isNull()) {
8183
THROW_ARANGO_EXCEPTION_MESSAGE(
8284
TRI_ERROR_BAD_PARAMETER,
8385
"invalid non-string value for 'distributeShardsLike'");
@@ -501,6 +503,16 @@ Result ShardingInfo::validateShardsAndReplicationFactor(arangodb::velocypack::Sl
501503

502504
TRI_ASSERT((cl.forceOneShard() && numberOfShards <= 1) || !cl.forceOneShard());
503505
}
506+
507+
auto writeConcernSlice = slice.get(StaticStrings::WriteConcern);
508+
auto minReplicationFactorSlice = slice.get(StaticStrings::MinReplicationFactor);
509+
510+
if (writeConcernSlice.isNumber() && minReplicationFactorSlice.isNumber()) {
511+
// both attributes set. now check if they have different values
512+
if (basics::VelocyPackHelper::compare(writeConcernSlice, minReplicationFactorSlice, false) != 0) {
513+
return Result(TRI_ERROR_BAD_PARAMETER, "got ambiguous values for writeConcern and minReplicationFactor");
514+
}
515+
}
504516

505517
if (enforceReplicationFactor) {
506518
auto enforceSlice = slice.get("enforceReplicationFactor");
@@ -536,9 +548,8 @@ Result ShardingInfo::validateShardsAndReplicationFactor(arangodb::velocypack::Sl
536548

537549
if (!replicationFactorSlice.isString()) {
538550
// beware: "satellite" replicationFactor
539-
auto writeConcernSlice = slice.get(StaticStrings::WriteConcern);
540551
if (writeConcernSlice.isNone()) {
541-
writeConcernSlice = slice.get(StaticStrings::MinReplicationFactor);
552+
writeConcernSlice = minReplicationFactorSlice;
542553
}
543554

544555
if (writeConcernSlice.isNumber()) {

‎arangod/VocBase/Methods/Collections.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,8 @@ Result Collections::create(TRI_vocbase_t& vocbase,
310310
auto distributeSlice = info.properties.get(StaticStrings::DistributeShardsLike);
311311
if (distributeSlice.isNone()) {
312312
helper.add(StaticStrings::DistributeShardsLike, VPackValue(vocbase.shardingPrototypeName()));
313-
} else if (distributeSlice.isString() && distributeSlice.compareString("") == 0) {
313+
} else if (distributeSlice.isNull() ||
314+
(distributeSlice.isString() && distributeSlice.compareString("") == 0)) {
314315
helper.add(StaticStrings::DistributeShardsLike, VPackSlice::nullSlice()); //delete empty string from info slice
315316
}
316317
}

‎js/common/modules/jsunity/jsunity.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -564,12 +564,12 @@ var jsUnity = exports.jsUnity = (function () {
564564
}
565565
break;
566566
} catch (e) {
567+
let arangodb = require("@arangodb");
567568
if ( typeof e === "string" ) {
568569
e = new Error(e);
569-
}
570-
else if (e instanceof require("@arangodb").ArangoError && (
571-
(e.errorNum === arangodb.errors.ERROR_CLUSTER_TIMEOUT) ||
572-
(e.errorNum === arangodb.errors.ERROR_LOCK_TIMEOUT)
570+
} else if (e instanceof arangodb.ArangoError && (
571+
(e.errorNum === arangodb.errors.ERROR_CLUSTER_TIMEOUT) ||
572+
(e.errorNum === arangodb.errors.ERROR_LOCK_TIMEOUT)
573573
)) {
574574
skipTest = true;
575575
}

‎tests/js/common/shell/shell-one-shard.js

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ function OneShardPropertiesSuite () {
5555
} catch (ex) {
5656
}
5757
},
58-
58+
5959
testDefaultValues : function () {
6060
assertTrue(db._createDatabase(dn));
6161
db._useDatabase(dn);
@@ -69,7 +69,28 @@ function OneShardPropertiesSuite () {
6969
}
7070
},
7171

72-
testShardingFlexibles : function () {
72+
testDefaultValuesOverridden : function () {
73+
assertTrue(db._createDatabase(dn, { replicationFactor: 2, writeConcern: 2, sharding: "single" }));
74+
db._useDatabase(dn);
75+
let props = db._properties();
76+
if (isCluster) {
77+
assertEqual(props.sharding, "single");
78+
assertEqual(props.replicationFactor, 2);
79+
assertEqual(props.writeConcern, 2);
80+
81+
let c = db._create("test", { writeConcern: 1, replicationFactor: 1, numberOfShards: 1, distributeShardsLike: "" });
82+
props = c.properties();
83+
assertEqual(1, props.writeConcern);
84+
assertEqual(1, props.replicationFactor);
85+
assertEqual(1, props.numberOfShards);
86+
} else {
87+
assertEqual(props.sharding, undefined);
88+
assertEqual(props.replicationFactor, undefined);
89+
assertEqual(props.writeConcern, undefined);
90+
}
91+
},
92+
93+
testShardingFlexible : function () {
7394
assertTrue(db._createDatabase(dn, { sharding: "flexible" }));
7495
db._useDatabase(dn);
7596
let props = db._properties();
@@ -81,6 +102,36 @@ function OneShardPropertiesSuite () {
81102
assertEqual(props.replicationFactor, undefined);
82103
}
83104
},
105+
106+
testDeviatingWriteConcernAndMinReplicationFactorForDatabase : function () {
107+
if (!isCluster) {
108+
return;
109+
}
110+
try {
111+
db._createDatabase(dn, { replicationFactor: 2, minReplicationFactor: 1, writeConcern: 2, sharding: "flexible" });
112+
fail();
113+
} catch (err) {
114+
assertEqual(ERRORS.ERROR_BAD_PARAMETER.code, err.errorNum);
115+
}
116+
117+
assertTrue(db._createDatabase(dn, { replicationFactor: 2, minReplicationFactor: 2, writeConcern: 2, sharding: "flexible" }));
118+
},
119+
120+
testDeviatingWriteConcernAndMinReplicationFactorForCollection : function () {
121+
if (!isCluster) {
122+
return;
123+
}
124+
assertTrue(db._createDatabase(dn, { sharding: "flexible" }));
125+
db._useDatabase(dn);
126+
try {
127+
db._create("test", { replicationFactor: 2, minReplicationFactor: 1, writeConcern: 2 });
128+
fail();
129+
} catch (err) {
130+
assertEqual(ERRORS.ERROR_BAD_PARAMETER.code, err.errorNum);
131+
}
132+
133+
db._create("test", { replicationFactor: 2, minReplicationFactor: 2, writeConcern: 2 });
134+
},
84135

85136
testNormalDBAndTooManyServers : function () {
86137
if (!isCluster) {

0 commit comments

Comments
 (0)
0