From 64ac6d7af3e6850784defbb18fe9aeea4b3de1da Mon Sep 17 00:00:00 2001 From: Lars Maier Date: Wed, 2 Oct 2019 10:24:54 +0200 Subject: [PATCH] Bug fix 3.5/backup list with bad dbserver (#10130) * Added available field to indicate bad backups. * Added nrPiecesPresent. * Fix logids. * Make Windows compilation happy. * Fix log ids. --- arangod/Cluster/ClusterMethods.cpp | 58 +++++++++++++------------ arangod/StorageEngine/HotBackupCommon.h | 14 +++++- arangosh/Backup/BackupFeature.cpp | 6 +++ 3 files changed, 50 insertions(+), 28 deletions(-) diff --git a/arangod/Cluster/ClusterMethods.cpp b/arangod/Cluster/ClusterMethods.cpp index 8c07ac3c11..6b829df717 100644 --- a/arangod/Cluster/ClusterMethods.cpp +++ b/arangod/Cluster/ClusterMethods.cpp @@ -3232,6 +3232,10 @@ arangodb::Result hotBackupList(std::vector const& dbServers, VPackSlic // Now check results for (auto const& req : requests) { auto res = req.result; + if (res.answer == NULL) { + continue; + } + TRI_ASSERT(res.answer != nullptr); auto resBody = res.answer->toVelocyPackBuilderPtrNoUniquenessChecks(); VPackSlice resSlice = resBody->slice(); @@ -3280,38 +3284,38 @@ arangodb::Result hotBackupList(std::vector const& dbServers, VPackSlic for (auto& i : dbsBackups) { // check if the backup is on all dbservers - if (i.second.size() == dbServers.size()) { - bool valid = true; + bool valid = true; - // check here that the backups are all made with the same version - std::string version; - size_t totalSize = 0; - size_t totalFiles = 0; + // check here that the backups are all made with the same version + std::string version; + size_t totalSize = 0; + size_t totalFiles = 0; - for (BackupMeta const& meta : i.second) { - if (version.empty()) { - version = meta._version; - } else { - if (version != meta._version) { - LOG_TOPIC("aaaaa", WARN, Logger::BACKUP) - << "Backup " << meta._id - << " has different versions accross dbservers: " << version - << " and " << meta._version; - valid = false; - break; - } + for (BackupMeta const& meta : i.second) { + if (version.empty()) { + version = meta._version; + } else { + if (version != meta._version) { + LOG_TOPIC("aaaaa", WARN, Logger::BACKUP) + << "Backup " << meta._id + << " has different versions accross dbservers: " << version + << " and " << meta._version; + valid = false; + break; } - totalSize += meta._sizeInBytes; - totalFiles += meta._nrFiles; } + totalSize += meta._sizeInBytes; + totalFiles += meta._nrFiles; + } - if (valid) { - BackupMeta& front = i.second.front(); - front._sizeInBytes = totalSize; - front._nrFiles = totalFiles; - front._serverId = ""; // makes no sense for whole cluster - hotBackups.insert(std::make_pair(front._id, front)); - } + if (valid) { + BackupMeta& front = i.second.front(); + front._sizeInBytes = totalSize; + front._nrFiles = totalFiles; + front._serverId = ""; // makes no sense for whole cluster + front._isAvailable = i.second.size() == dbServers.size(); + front._nrPiecesPresent = static_cast(i.second.size()); + hotBackups.insert(std::make_pair(front._id, front)); } } diff --git a/arangod/StorageEngine/HotBackupCommon.h b/arangod/StorageEngine/HotBackupCommon.h index 0ff08ffb57..f2ee203f03 100644 --- a/arangod/StorageEngine/HotBackupCommon.h +++ b/arangod/StorageEngine/HotBackupCommon.h @@ -51,6 +51,8 @@ struct BackupMeta { unsigned int _nrDBServers; std::string _serverId; bool _potentiallyInconsistent; + bool _isAvailable; + unsigned int _nrPiecesPresent; static constexpr const char *ID = "id"; static constexpr const char *VERSION = "version"; @@ -60,6 +62,9 @@ struct BackupMeta { static constexpr const char *NRDBSERVERS = "nrDBServers"; static constexpr const char *SERVERID = "serverId"; static constexpr const char *POTENTIALLYINCONSISTENT = "potentiallyInconsistent"; + static constexpr const char *AVAILABLE = "available"; + static constexpr const char *NRPIECESPRESENT = "nrPiecesPresent"; + void toVelocyPack(VPackBuilder &builder) const { { @@ -73,6 +78,10 @@ struct BackupMeta { if (ServerState::instance()->isDBServer()) { builder.add(SERVERID, VPackValue(_serverId)); } + if (ServerState::instance()->isCoordinator() || ServerState::instance()->isSingleServer()) { + builder.add(AVAILABLE, VPackValue(_isAvailable)); + builder.add(NRPIECESPRESENT, VPackValue(_nrPiecesPresent)); + } builder.add(POTENTIALLYINCONSISTENT, VPackValue(_potentiallyInconsistent)); } } @@ -91,6 +100,9 @@ struct BackupMeta { slice, NRDBSERVERS, 1); meta._serverId = basics::VelocyPackHelper::getStringValue(slice, SERVERID, ""); meta._potentiallyInconsistent = basics::VelocyPackHelper::getBooleanValue(slice, POTENTIALLYINCONSISTENT, false); + meta._isAvailable = basics::VelocyPackHelper::getBooleanValue(slice, AVAILABLE, true); + meta._nrPiecesPresent = basics::VelocyPackHelper::getNumericValue( + slice, NRPIECESPRESENT, 1); return meta; } catch (std::exception const& e) { return ResultT::error(TRI_ERROR_BAD_PARAMETER, e.what()); @@ -100,7 +112,7 @@ struct BackupMeta { BackupMeta(std::string const& id, std::string const& version, std::string const& datetime, size_t sizeInBytes, size_t nrFiles, unsigned int nrDBServers, std::string const& serverId, bool potentiallyInconsistent) : _id(id), _version(version), _datetime(datetime), _sizeInBytes(sizeInBytes), _nrFiles(nrFiles), _nrDBServers(nrDBServers), - _serverId(serverId), _potentiallyInconsistent(potentiallyInconsistent) {} + _serverId(serverId), _potentiallyInconsistent(potentiallyInconsistent),_isAvailable(true), _nrPiecesPresent(1) {} private: BackupMeta() {} diff --git a/arangosh/Backup/BackupFeature.cpp b/arangosh/Backup/BackupFeature.cpp index 9c089919be..20f1afca90 100644 --- a/arangosh/Backup/BackupFeature.cpp +++ b/arangosh/Backup/BackupFeature.cpp @@ -245,6 +245,7 @@ arangodb::Result executeList(arangodb::httpclient::SimpleHttpClient& client, LOG_TOPIC("43522", INFO, arangodb::Logger::BACKUP) << " size in bytes: " << meta.get()._sizeInBytes; LOG_TOPIC("12532", INFO, arangodb::Logger::BACKUP) << " number of files: " << meta.get()._nrFiles; LOG_TOPIC("43212", INFO, arangodb::Logger::BACKUP) << " number of DBServers: " << meta.get()._nrDBServers; + LOG_TOPIC("43213", INFO, arangodb::Logger::BACKUP) << " number of available pieces: " << meta.get()._nrPiecesPresent; if (!meta.get()._serverId.empty()) { LOG_TOPIC("11112", INFO, arangodb::Logger::BACKUP) << " serverId: " << meta.get()._serverId; } @@ -253,6 +254,11 @@ arangodb::Result executeList(arangodb::httpclient::SimpleHttpClient& client, } else { LOG_TOPIC("56242", INFO, arangodb::Logger::BACKUP) << " potentiallyInconsistent: false"; } + if (meta.get()._isAvailable) { + LOG_TOPIC("56244", INFO, arangodb::Logger::BACKUP) << " available: true"; + } else { + LOG_TOPIC("56246", INFO, arangodb::Logger::BACKUP) << " available: false"; + } } } }