8000 Reduce follower response sizes in replication (#16097) · OOSYOO/arangodb@70372f2 · GitHub
[go: up one dir, main page]
Skip to content

Commit 70372f2

Browse files
jsteemannjsteemann
authored
Reduce follower response sizes in replication (arangodb#16097)
1 parent 2e87504 commit 70372f2

File tree

5 files changed

+287
-25
lines changed

5 files changed

+287
-25
lines changed

‎CHANGELOG

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

4+
* Make followers respond to synchronous replication requests with less data.
5+
Specifically, followers will not build detailed results with _id, _key and
6+
_rev for the inserted/modified/removed documents, which would be ignored
7+
by the leader anyway.
8+
49
* Updated arangosync to v2.9.0-preview-5.
510

611
* Auto-regenerate exit code and error code files in non-maintainer mode, too.

‎arangod/Transaction/Methods.cpp

Lines changed: 103 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -215,21 +215,25 @@ static void throwCollectionNotFound(char const* name) {
215215

216216
/// @brief Insert an error reported instead of the new document
217217
static void createBabiesError(
218-
VPackBuilder& builder,
218+
VPackBuilder* builder,
219219
std::unordered_map<ErrorCode, size_t>& countErrorCodes,
220220
Result const& error) {
221-
builder.openObject();
222-
builder.add(StaticStrings::Error, VPackValue(true));
223-
builder.add(StaticStrings::ErrorNum, VPackValue(error.errorNumber()));
224-
builder.add(StaticStrings::ErrorMessage, VPackValue(error.errorMessage()));
225-
builder.close();
226-
227-
auto it = countErrorCodes.find(error.errorNumber());
228-
if (it == countErrorCodes.end()) {
229-
countErrorCodes.emplace(error.errorNumber(), 1);
230-
} else {
231-
it->second++;
221+
// on followers, builder will be a nullptr, so we can spare building
222+
// the error result details in the response body, which the leader
223+
// will ignore anyway.
224+
if (builder != nullptr) {
225+
// only build error detail results if we got a builder passed here.
226+
builder->openObject(false);
227+
builder->add(StaticStrings::Error, VPackValue(true));
228+
builder->add(StaticStrings::ErrorNum, VPackValue(error.errorNumber()));
229+
builder->add(StaticStrings::ErrorMessage, VPackValue(error.errorMessage()));
230+
builder->close();
232231
}
232+
233+
// always (also on followers) increase error counter for the
234+
// error code we got.
235+
auto& value = countErrorCodes[error.errorNumber()];
236+
++value;
233237
}
234238

235239
static OperationResult emptyResult(OperationOptions const& options) {
@@ -907,7 +911,7 @@ Future<OperationResult> transaction::Methods::documentLocal(
907911
for (VPackSlice s : VPackArrayIterator(value)) {
908912
res = workForOneDocument(s, true);
909913
if (res.fail()) {
910-
createBabiesError(resultBuilder, countErrorCodes, res);
914+
createBabiesError(&resultBuilder, countErrorCodes, res);
911915
}
912916
}
913917
res.reset(); // With babies the reporting is handled somewhere else.
@@ -1045,7 +1049,11 @@ Future<OperationResult> transaction::Methods::insertLocal(
10451049
return OperationResult(TRI_ERROR_CLUSTER_SHARD_LEADER_RESIGNED,
10461050
options);
10471051
}
1052+
10481053
bool sendRefusal = (options.isSynchronousReplicationFrom != theLeader);
1054+
TRI_IF_FAILURE("synchronousReplication::neverRefuseOnFollower") {
1055+
sendRefusal = false;
1056+
}
10491057
TRI_IF_FAILURE("synchronousReplication::refuseOnFollower") {
10501058
sendRefusal = true;
10511059
}
@@ -1061,6 +1069,12 @@ Future<OperationResult> transaction::Methods::insertLocal(
10611069
::buildRefusalResult(*collection, "insert", options, theLeader),
10621070
options);
10631071
}
1072+
1073+
// we are a valid follower. we do not need to send a proper result with
1074+
// _key, _id, _rev back to the leader, because it will ignore all these
1075+
// data anyway. it is sufficient to send headers and the proper error
1076+
// codes back.
1077+
options.silent = true;
10641078
}
10651079
} // isDBServer - early block
10661080

@@ -1248,13 +1262,19 @@ Future<OperationResult> transaction::Methods::insertLocal(
12481262
VPackSlice s = it.value();
12491263
TRI_IF_FAILURE("insertLocal::fakeResult1") {
12501264
res.reset(TRI_ERROR_DEBUG);
1251-
createBabiesError(resultBuilder, errorCounter, res);
1265+
createBabiesError(replicationType == ReplicationType::FOLLOWER
1266+
? nullptr
1267+
: &resultBuilder,
1268+
errorCounter, res);
12521269
it.next();
12531270
continue;
12541271
}
12551272
res = workForOneDocument(s, true, excludeFromReplication);
12561273
if (res.fail()) {
1257-
createBabiesError(resultBuilder, errorCounter, res);
1274+
createBabiesError(replicationType == ReplicationType::FOLLOWER
1275+
? nullptr
1276+
: &resultBuilder,
1277+
errorCounter, res);
12581278
} else if (excludeFromReplication) {
12591279
excludePositions.insert(it.index());
12601280
}
@@ -1269,8 +1289,21 @@ Future<OperationResult> transaction::Methods::insertLocal(
12691289
if (res.ok() && excludeFromReplication) {
12701290
excludePositions.insert(0);
12711291
}
1292+
1293+
// on a follower, our result should always be an empty object
1294+
if (replicationType == ReplicationType::FOLLOWER) {
1295+
TRI_ASSERT(resultBuilder.slice().isNone());
1296+
// add an empty object here so that when sending things back in JSON
1297+
// format, there is no "non-representable type 'none'" issue.
1298+
resultBuilder.add(VPackSlice::emptyObjectSlice());
1299+
}
12721300
}
12731301

1302+
// on a follower, our result should always be an empty array or object
1303+
TRI_ASSERT(replicationType != ReplicationType::FOLLOWER ||
1304+
(value.isArray() && resultBuilder.slice().isEmptyArray()) ||
1305+
(value.isObject() && resultBuilder.slice().isEmptyObject()));
1306+
12741307
TRI_ASSERT(res.ok() || !value.isArray());
12751308

12761309
TRI_IF_FAILURE("insertLocal::fakeResult2") { res.reset(TRI_ERROR_DEBUG); }
@@ -1438,6 +1471,9 @@ Future<OperationResult> transaction::Methods::modifyLocal(
14381471
options);
14391472
}
14401473
bool sendRefusal = (options.isSynchronousReplicationFrom != theLeader);
1474+
TRI_IF_FAILURE("synchronousReplication::neverRefuseOnFollower") {
1475+
sendRefusal = false;
1476+
}
14411477
TRI_IF_FAILURE("synchronousReplication::refuseOnFollower") {
14421478
sendRefusal = true;
14431479
}
@@ -1457,6 +1493,12 @@ Future<OperationResult> transaction::Methods::modifyLocal(
14571493
options, theLeader),
14581494
options);
14591495
}
1496+
1497+
// we are a valid follower. we do not need to send a proper result with
1498+
// _key, _id, _rev back to the leader, because it will ignore all these
1499+
// data anyway. it is sufficient to send headers and the proper error
1500+
// codes back.
1501+
options.silent = true;
14601502
}
14611503
} // isDBServer - early block
14621504

@@ -1548,7 +1590,10 @@ Future<OperationResult> transaction::Methods::modifyLocal(
15481590
bool excludeFromReplication = false;
15491591
res = workForOneDocument(it.value(), true, excludeFromReplication);
15501592
if (res.fail()) {
1551-
createBabiesError(resultBuilder, errorCounter, res);
1593+
createBabiesError(replicationType == ReplicationType::FOLLOWER
1594+
? nullptr
1595+
: &resultBuilder,
1596+
errorCounter, res);
15521597
} else if (excludeFromReplication) {
15531598
excludePositions.insert(it.index());
15541599
}
@@ -1562,8 +1607,21 @@ Future<OperationResult> transaction::Methods::modifyLocal(
15621607
if (res.ok() && excludeFromReplication) {
15631608
excludePositions.insert(0);
15641609
}
1610+
1611+
// on a follower, our result should always be an empty object
1612+
if (replicationType == ReplicationType::FOLLOWER) {
1613+
TRI_ASSERT(resultBuilder.slice().isNone());
1614+
// add an empty object here so that when sending things back in JSON
1615+
// format, there is no "non-representable type 'none'" issue.
1616+
resultBuilder.add(VPackSlice::emptyObjectSlice());
1617+
}
15651618
}
15661619

1620+
// on a follower, our result should always be an empty array or object
1621+
TRI_ASSERT(replicationType != ReplicationType::FOLLOWER ||
1622+
(newValue.isArray() && resultBuilder.slice().isEmptyArray()) ||
1623+
(newValue.isObject() && resultBuilder.slice().isEmptyObject()));
1624+
15671625
TRI_ASSERT(!newValue.isArray() || options.silent ||
15681626
resultBuilder.slice().length() == newValue.length());
15691627

@@ -1707,6 +1765,9 @@ Future<OperationResult> transaction::Methods::removeLocal(
17071765
options);
17081766
}
17091767
bool sendRefusal = (options.isSynchronousReplicationFrom != theLeader);
1768+
TRI_IF_FAILURE("synchronousReplication::neverRefuseOnFollower") {
1769+
sendRefusal = false;
1770+
}
17101771
TRI_IF_FAILURE("synchronousReplication::refuseOnFollower") {
17111772
sendRefusal = true;
17121773
}
@@ -1722,6 +1783,12 @@ Future<OperationResult> transaction::Methods::removeLocal(
17221783
::buildRefusalResult(*collection, "remove", options, theLeader),
17231784
options);
17241785
}
1786+
1787+
// we are a valid follower. we do not need to send a proper result with
1788+
// _key, _id, _rev back to the leader, because it will ignore all these
1789+
// data anyway. it is sufficient to send headers and the proper error
1790+
// codes back.
1791+
options.silent = true;
17251792
}
17261793
} // isDBServer - early block
17271794

@@ -1792,16 +1859,32 @@ Future<OperationResult> transaction::Methods::removeLocal(
17921859
for (VPackSlice s : VPackArrayIterator(value)) {
17931860
res = workForOneDocument(s, true);
17941861
if (res.fail()) {
1795-
createBabiesError(resultBuilder, errorCounter, res);
1862+
createBabiesError(replicationType == ReplicationType::FOLLOWER
1863+
? nullptr
1864+
: &resultBuilder,
1865+
errorCounter, res);
17961866
}
17971867
}
17981868

17991869
// it is ok to clobber res here!
18001870
res.reset();
18011871
} else {
18021872
res = workForOneDocument(value, false);
1873+
1874+
// on a follower, our result should always be an empty object
1875+
if (replicationType == ReplicationType::FOLLOWER) {
1876+
TRI_ASSERT(resultBuilder.slice().isNone());
1877+
// add an empty object here so that when sending things back in JSON
1878+
// format, there is no "non-representable type 'none'" issue.
1879+
resultBuilder.add(VPackSlice::emptyObjectSlice());
1880+
}
18031881
}
18041882

1883+
// on a follower, our result should always be an empty array or object
1884+
TRI_ASSERT(replicationType != ReplicationType::FOLLOWER ||
1885+
(value.isArray() && resultBuilder.slice().isEmptyArray()) ||
1886+
(value.isObject() && resultBuilder.slice().isEmptyObject()));
1887+
18051888
TRI_ASSERT(!value.isArray() || options.silent ||
18061889
resultBuilder.slice().length() == value.length());
18071890

@@ -1969,6 +2052,9 @@ Future<OperationResult> transaction::Methods::truncateLocal(
19692052
OperationResult(TRI_ERROR_CLUSTER_SHARD_LEADER_RESIGNED, options));
19702053
}
19712054
bool sendRefusal = (options.isSynchronousReplicationFrom != theLeader);
2055+
TRI_IF_FAILURE("synchronousReplication::neverRefuseOnFollower") {
2056+
sendRefusal = false;
2057+
}
19722058
TRI_IF_FAILURE("synchronousReplication::refuseOnFollower") {
19732059
sendRefusal = true;
19742060
}

0 commit comments

Comments
 (0)
0