From 56eaf1bc839f2d58fa81b97f39677749dfd1dade Mon Sep 17 00:00:00 2001 From: Markus Pilman Date: Tue, 15 Jun 2021 16:49:27 -0600 Subject: [PATCH] added comments --- fdbserver/QuietDatabase.actor.cpp | 5 +++++ fdbserver/SimulatedCluster.actor.cpp | 11 +++++++---- fdbserver/worker.actor.cpp | 6 ++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/fdbserver/QuietDatabase.actor.cpp b/fdbserver/QuietDatabase.actor.cpp index 8f05a0d054..16598d6639 100644 --- a/fdbserver/QuietDatabase.actor.cpp +++ b/fdbserver/QuietDatabase.actor.cpp @@ -284,6 +284,8 @@ ACTOR Future> getStorageWorkers(Database cx, return result; } +// Helper function to extract he maximum SQ size of all provided messages. All futures in the +// messages vector have to be ready. int64_t extractMaxQueueSize(const std::vector>& messages, const std::vector& servers) { @@ -315,6 +317,7 @@ int64_t extractMaxQueueSize(const std::vector>& message return maxQueueSize; } +// Timeout wrapper when getting the storage metrics. This will do some additional tracing ACTOR Future getStorageMetricsTimeout(UID storage, WorkerInterface wi) { state Future result = wi.eventLogRequest.getReply(EventLogRequest(StringRef(storage.toString() + "/StorageMetrics"))); @@ -608,6 +611,8 @@ ACTOR Future reconfigureAfter(Database cx, return Void(); } +// Waits until a database quiets down (no data in flight, small tlog queue, low SQ, no active data distribution). This +// requires the database to be available and healthy in order to succeed. ACTOR Future waitForQuietDatabase(Database cx, Reference> dbInfo, std::string phase, diff --git a/fdbserver/SimulatedCluster.actor.cpp b/fdbserver/SimulatedCluster.actor.cpp index 0121a22338..ce0af1d345 100644 --- a/fdbserver/SimulatedCluster.actor.cpp +++ b/fdbserver/SimulatedCluster.actor.cpp @@ -53,17 +53,19 @@ const int MACHINE_REBOOT_TIME = 10; bool destructed = false; +// given a std::variant T, this templated class will define type which will be a variant of all types in T plus Args... template -struct concatenate_variant_t; +struct variant_add_t; template -struct concatenate_variant_t, Args2...> { +struct variant_add_t, Args2...> { using type = std::variant; }; template -using concatenate_variant = typename concatenate_variant_t::type; +using variant_add = typename variant_add_t::type; +// variant_with_optional_t::type will be a std::variant...> template struct variant_with_optional_t; @@ -74,12 +76,13 @@ struct variant_with_optional_t { template struct variant_with_optional_t { - using type = concatenate_variant::type, Head, Optional>; + using type = variant_add::type, Head, Optional>; }; template using variant_with_optional = typename variant_with_optional_t::type; +// Expects a std::variant and will make a pointer out of each argument template struct add_pointers_t; diff --git a/fdbserver/worker.actor.cpp b/fdbserver/worker.actor.cpp index a2ab8f8ce2..bea37b70fa 100644 --- a/fdbserver/worker.actor.cpp +++ b/fdbserver/worker.actor.cpp @@ -1536,6 +1536,12 @@ ACTOR Future workerServer(Reference connFile, activeSharedTLog->set(logData.uid); } when(InitializeStorageRequest req = waitNext(interf.storage.getFuture())) { + // We want to prevent double recruiting on a worker unless we try to recruit something + // with a different storage engine (otherwise storage migration won't work for certain + // configuration). Additionally we also need to allow double recruitment for seed servers. + // The reason for this is that a storage will only remove itself if after it was able + // to read the system key space. But if recovery fails right after a `configure new ...` + // was run it won't be able to do so. if (!storageCache.exists(req.reqId) && (std::all_of(runningStorages.begin(), runningStorages.end(),