8000 [3.3] Backport #6539. (#6569) · mnemosdev/arangodb@069b456 · GitHub
[go: up one dir, main page]
Skip to content

Commit 069b456

Browse files
Dan Larkin-Yorkjsteemann
Dan Larkin-York
authored and
jsteemann
committed
[3.3] Backport arangodb#6539. (arangodb#6569)
1 parent e1bac93 commit 069b456

File tree

19 files changed

+896
-422
lines changed

19 files changed

+896
-422
lines changed

‎arangod/Cluster/ClusterInfo.cpp

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2623,33 +2623,26 @@ void ClusterInfo::loadCurrentMappings() {
26232623

26242624
if (mappings.isObject()) {
26252625
decltype(_coordinatorIdMap) newCoordinatorIdMap;
2626-
decltype(_dbserverIdMap) newDBServerIdMap;
2627-
decltype(_nameMap) newNameMap;
26282626

26292627
for (auto const& mapping : VPackObjectIterator(mappings)) {
26302628
ServerID fullId = mapping.key.copyString();
26312629
auto mapObject = mapping.value;
26322630
if (mapObject.isObject()) {
26332631
ServerShortName shortName = mapObject.get("ShortName").copyString();
2634-
newNameMap.emplace(shortName, fullId);
26352632

26362633
ServerShortID shortId = mapObject.get("TransactionID").getNumericValue<ServerShortID>();
26372634
static std::string const expectedPrefix{"Coordinator"};
26382635
if (shortName.size() > expectedPrefix.size() &&
26392636
shortName.substr(0, expectedPrefix.size()) == expectedPrefix) {
26402637
newCoordinatorIdMap.emplace(shortId, fullId);
2641-
} else {
2642-
newDBServerIdMap.emplace(shortId, fullId);
26432638
}
26442639
}
26452640
}
26462641

26472642
// Now set the new value:
26482643
{
26492644
WRITE_LOCKER(writeLocker, _mappingsProt.lock);
2650-
_nameMap.swap(newNameMap);
26512645
_coordinatorIdMap.swap(newCoordinatorIdMap);
2652-
_dbserverIdMap.swap(newDBServerIdMap);
26532646
_mappingsProt.doneVersion = storedVersion;
26542647
_mappingsProt.isValid = true;
26552648
}
@@ -2993,50 +2986,6 @@ ServerID ClusterInfo::getCoordinatorByShortID(ServerShortID shortId) {
29932986
return result;
29942987
}
29952988

2996-
////////////////////////////////////////////////////////////////////////////////
2997-
/// @brief lookup full dbserver ID from short ID
2998-
////////////////////////////////////////////////////////////////////////////////
2999-
3000-
ServerID ClusterInfo::getDBServerByShortID(ServerShortID shortId) {
3001-
ServerID result;
3002-
3003-
if (!_mappingsProt.isValid) {
3004-
loadCurrentMappings();
3005-
}
3006-
3007-
// return a consistent state of servers
3008-
READ_LOCKER(readLocker, _mappingsProt.lock);
3009-
3010-
auto it = _dbserverIdMap.find(shortId);
3011-
if (it != _dbserverIdMap.end()) {
3012-
result = it->second;
3013-
}
3014-
3015-
return result;
3016-
}
3017-
3018-
////////////////////////////////////////////////////////////////////////////////
3019-
/// @brief lookup full server ID from short name
3020-
////////////////////////////////////////////////////////////////////////////////
3021-
3022-
ServerID ClusterInfo::getServerByShortName(ServerShortName const& shortName) {
3023-
ServerID result;
3024-
3025-
if (!_mappingsProt.isValid) {
3026-
loadCurrentMappings();
3027-
}
3028-
3029-
// return a consistent state of servers
3030-
READ_LOCKER(readLocker, _mappingsProt.lock);
3031-
3032-
auto it = _nameMap.find(shortName);
3033-
if (it != _nameMap.end()) {
3034-
result = it->second;
3035-
}
3036-
3037-
return result;
3038-
}
3039-
30402989
//////////////////////////////////////////////////////////////////////////////
30412990
/// @brief invalidate plan
30422991
//////////////////////////////////////////////////////////////////////////////

‎arangod/Cluster/ClusterInfo.h

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -499,18 +499,6 @@ class ClusterInfo {
499499

500500
ServerID getCoordinatorByShortID(ServerShortID);
501501

502-
//////////////////////////////////////////////////////////////////////////////
503-
/// @brief lookup a full dbserver ID by short ID
504-
//////////////////////////////////////////////////////////////////////////////
505-
506-
ServerID getDBServerByShortID(ServerShortID);
507-
508-
//////////////////////////////////////////////////////////////////////////////
509-
/// @brief lookup a full server ID by short name
510-
//////////////////////////////////////////////////////////////////////////////
511-
512-
ServerID getServerByShortName(ServerShortName const&);
513-
514502
//////////////////////////////////////////////////////////////////////////////
515503
/// @brief invalidate planned
516504
//////////////////////////////////////////////////////////////////////////////
@@ -644,8 +632,6 @@ class ClusterInfo {
644632

645633
// Mappings between short names/IDs and full server IDs
646634
std::unordered_map<ServerShortID, ServerID> _coordinatorIdMap;
647-
std::unordered_map<ServerShortID, ServerID> _dbserverIdMap;
648-
std::unordered_map<ServerShortName, ServerID> _nameMap;
649635
ProtectionData _mappingsProt;
650636

651637
std::shared_ptr<VPackBuilder> _plan;

‎arangod/MMFiles/MMFilesCollection.cpp

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2323,7 +2323,7 @@ int MMFilesCollection::restoreIndex(transaction::Methods* trx,
23232323
if (newIdx == nullptr) {
23242324
return TRI_ERROR_ARANGO_INDEX_CREATION_FAILED;
23252325
}
2326-
2326+
23272327
auto const id = newIdx->id();
23282328
TRI_UpdateTickServer(id);
23292329
for (auto& it : _indexes) {
@@ -2734,8 +2734,8 @@ int MMFilesCollection::unlockWrite(bool useDeadlockDetector) {
27342734
return TRI_ERROR_NO_ERROR;
27352735
}
27362736

2737-
void MMFilesCollection::truncate(transaction::Methods* trx,
2738-
OperationOptions& options) {
2737+
Result MMFilesCollection::truncate(transaction::Methods* trx,
2738+
OperationOptions& options) {
27392739
auto primaryIdx = primaryIndex();
27402740

27412741
options.ignoreRevs = true;
@@ -2764,8 +2764,16 @@ void MMFilesCollection::truncate(transaction::Methods* trx,
27642764

27652765
return true;
27662766
};
2767-
primaryIdx->invokeOnAllElementsForRemoval(callback);
2768-
2767+
try {
2768+
primaryIdx->invokeOnAllElementsForRemoval(callback);
2769+
} catch(basics::Exception const& e) {
2770+
return Result(e.code(), e.message());
2771+
} catch(std::exception const& e) {
2772+
return Result(TRI_ERROR_INTERNAL, e.what());
2773+
} catch(...) {
2774+
return Result(TRI_ERROR_INTERNAL, "unknown error during truncate");
2775+
}
2776+
27692777
auto indexes = _indexes;
27702778
size_t const n = indexes.size();
27712779

@@ -2774,6 +2782,8 @@ void MMFilesCollection::truncate(transaction::Methods* trx,
27742782
TRI_ASSERT(idx->type() != Index::IndexType::TRI_IDX_TYPE_PRIMARY_INDEX);
27752783
idx->afterTruncate();
27762784
}
2785+
2786+
return Result();
27772787
}
27782788

27792789
Result MMFilesCollection::insert(transaction::Methods* trx,
@@ -2910,7 +2920,7 @@ Result MMFilesCollection::insert(transaction::Methods* trx,
29102920
operation.revert(trx);
29112921
}
29122922
}
2913-
2923+
29142924
if (res.ok()) {
29152925
uint8_t const* vpack = lookupDocumentVPack(documentId);
29162926
if (vpack != nullptr) {
@@ -3277,7 +3287,7 @@ Result MMFilesCollection::update(
32773287
documentId, options.mergeObjects,
32783288
options.keepNull, *builder.get(), options.isRestore, revisionId);
32793289

3280-
if (res.fail()) {
3290+
if (res.fail()) {
32813291
return res;
32823292
}
32833293

‎arangod/MMFiles/MMFilesCollection.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ class MMFilesCollection final : public PhysicalCollection {
327327
// -- SECTION DML Operations --
328328
///////////////////////////////////
329329

330-
void truncate(transaction::Methods* trx, OperationOptions& options) override;
330+
Result truncate(transaction::Methods* trx, OperationOptions& options) override;
331331

332332
LocalDocumentId lookupKey(transaction::Methods* trx,
333333
velocypack::Slice const& key) override;

‎arangod/RestHandler/RestReplicationHandler.cpp

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2016,16 +2016,19 @@ void RestReplicationHandler::handleCommandAddFollower() {
20162016
"and 'shard'");
20172017
return;
20182018
}
2019-
VPackSlice const followerId = body.get("followerId");
2020-
VPackSlice const readLockId = body.get("readLockId");
2021-
VPackSlice const shard = body.get("shard");
2022-
if (!followerId.isString() || !shard.isString()) {
2019+
VPackSlice const followerIdSlice = body.get("followerId");
2020+
VPackSlice const readLockIdSlice = body.get("readLockId");
2021+
VPackSlice const shardSlice = body.get("shard");
2022+
VPackSlice const checksumSlice = body.get("checksum");
2023+
if (!followerIdSlice.isString() ||
2024+
!shardSlice.isString() ||
2025+
!checksumSlice.isString()) {
20232026
generateError(rest::ResponseCode::BAD, TRI_ERROR_HTTP_BAD_PARAMETER,
2024-
"'followerId' and 'shard' attributes must be strings");
2027+
"'followerId', 'shard', and 'checksum' attributes must be strings");
20252028
return;
20262029
}
20272030

2028-
auto col = _vocbase->lookupCollection(shard.copyString());
2031+
auto col = _vocbase->lookupCollection(shardSlice.copyString());
20292032

20302033
if (col == nullptr) {
20312034
generateError(rest::ResponseCode::SERVER_ERROR,
@@ -2034,8 +2037,9 @@ void RestReplicationHandler::handleCommandAddFollower() {
20342037
return;
20352038
}
20362039

2037-
if (readLockId.isNone()) {
2038-
// Short cut for the case that the collection is empty
2040+
const std::string followerId = followerIdSlice.copyString();
2041+
// Short cut for the case that the collection is empty
2042+
if (readLockIdSlice.isNone()) {
20392043
auto ctx = transaction::StandaloneContext::Create(_vocbase);
20402044
SingleCollectionTransaction trx(ctx, col->cid(),
20412045
AccessMode::Type::EXCLUSIVE);
@@ -2046,8 +2050,8 @@ void RestReplicationHandler::handleCommandAddFollower() {
20462050
if (countRes.ok()) {
20472051
VPackSlice nrSlice = countRes.slice();
20482052
uint64_t nr = nrSlice.getNumber<uint64_t>();
2049-
if (nr == 0) {
2050-
col->followers()->add(followerId.copyString());
2053+
if (nr == 0 && checksumSlice.isEqualString("0")) {
2054+
col->followers()->add(followerId);
20512055

20522056
VPackBuilder b;
20532057
{
@@ -2068,15 +2072,14 @@ void RestReplicationHandler::handleCommandAddFollower() {
20682072
return;
20692073
}
20702074

2071-
VPackSlice const checksum = body.get("checksum");
20722075
// optional while introducing this bugfix. should definitely be required with
20732076
// 3.4
20742077
// and throw a 400 then when no checksum is provided
2075-
if (checksum.isString() && readLockId.isString()) {
2078+
if (readLockIdSlice.isString()) {
20762079
std::string referenceChecksum;
20772080
{
20782081
CONDITION_LOCKER(locker, _condVar);
2079-
auto it = _holdReadLockJobs.find(readLockId.copyString());
2082+
auto it = _holdReadLockJobs.find(readLockIdSlice.copyString());
20802083
if (it == _holdReadLockJobs.end()) {
20812084
// Entry has been removed since, so we cancel the whole thing
20822085
// right away and generate an error:
@@ -2100,23 +2103,19 @@ void RestReplicationHandler::handleCommandAddFollower() {
21002103
referenceChecksum = std::to_string(num);
21012104
}
21022105

2103-
auto result = col->compareChecksums(checksum, referenceChecksum);
2104-
2105-
if (result.fail()) {
2106-
auto errorNumber = result.errorNumber();
2107-
rest::ResponseCode code;
2108-
if (errorNumber == TRI_ERROR_REPLICATION_WRONG_CHECKSUM ||
2109-
errorNumber == TRI_ERROR_REPLICATION_WRONG_CHECKSUM_FORMAT) {
2110-
code = rest::ResponseCode::BAD;
2111-
} else {
2112-
code = rest::ResponseCode::SERVER_ERROR;
2113-
}
2114-
generateError(code, errorNumber, result.errorMessage());
2106+
if (!checksumSlice.isEqualString(referenceChecksum)) {
2107+
const std::string checksum = checksumSlice.copyString();
2108+
LOG_TOPIC(WARN, Logger::REPLICATION) << "Cannot add follower, mismatching checksums. "
2109+
<< "Expected: " << referenceChecksum << " Actual: " << checksum;
2110+
generateError(rest::ResponseCode::BAD, TRI_ERROR_REPLICATION_WRONG_CHECKSUM,
2111+
"'checksum' is wrong. Expected: "
2112+
+ referenceChecksum
2113+
+ ". Actual: " + checksum);
21152114
return;
21162115
}
21172116
}
21182117

2119-
col->followers()->add(followerId.copyString());
2118+
col->followers()->add(followerId);
21202119

21212120
VPackBuilder b;
21222121
{

0 commit comments

Comments
 (0)
0