Address/remove some TODO comments

This commit is contained in:
sfc-gh-tclinkenbeard 2021-05-18 18:32:56 -07:00
parent 818b0325ad
commit 3d97d92acf
7 changed files with 66 additions and 36 deletions

View File

@ -31,9 +31,8 @@ bool matchesConfigClass(Optional<ConfigClassSet> const& configClassSet, Optional
} // namespace
class ConfigBroadcasterImpl {
std::map<Key, Endpoint::Token> configClassToToken;
std::map<Endpoint::Token, ReplyPromise<ConfigFollowerGetChangesRequest>> tokenToReply;
std::map<Endpoint::Token, std::vector<Key>> tokenToConfigClasses;
std::map<Key, std::vector<Endpoint::Token>> configClassToTokens;
std::map<Endpoint::Token, ConfigFollowerGetChangesRequest> tokenToRequest;
std::map<ConfigKey, Value> snapshot;
std::deque<Standalone<VersionedConfigMutationRef>> versionedMutations;
Version lastCompactedVersion;
@ -44,11 +43,32 @@ class ConfigBroadcasterImpl {
UID id;
CounterCollection cc;
Counter compactRequest;
Counter successfulChangeRequest;
mutable Counter successfulChangeRequest;
Counter failedChangeRequest;
Counter snapshotRequest;
Future<Void> logger;
template <class Changes>
void sendChangesReply(ConfigFollowerGetChangesRequest const& req, Changes const& changes) const {
ASSERT_LT(req.lastSeenVersion, mostRecentVersion);
ConfigFollowerGetChangesReply reply;
reply.mostRecentVersion = mostRecentVersion;
for (const auto& versionedMutation : changes) {
if (versionedMutation.version > req.lastSeenVersion &&
matchesConfigClass(req.configClassSet, versionedMutation.mutation.getConfigClass())) {
TraceEvent(SevDebug, "ConfigBroadcasterSendingChangeMutation", id)
.detail("Version", versionedMutation.version)
.detail("ReqLastSeenVersion", req.lastSeenVersion)
.detail("ConfigClass", versionedMutation.mutation.getConfigClass())
.detail("KnobName", versionedMutation.mutation.getKnobName())
.detail("KnobValue", versionedMutation.mutation.getValue());
reply.versionedMutations.push_back_deep(reply.versionedMutations.arena(), versionedMutation);
}
}
req.reply.send(reply);
++successfulChangeRequest;
}
ACTOR static Future<Void> serve(ConfigBroadcaster* self, ConfigBroadcasterImpl* impl, ConfigFollowerInterface cfi) {
impl->actors.add(impl->consumer->consume(*self));
loop {
@ -70,29 +90,19 @@ class ConfigBroadcasterImpl {
++impl->failedChangeRequest;
continue;
}
// TODO: If there are no new changes, register the request and push
// changes when ready
ConfigFollowerGetChangesReply reply;
reply.mostRecentVersion = impl->mostRecentVersion;
for (const auto& versionedMutation : impl->versionedMutations) {
if (versionedMutation.version > req.lastSeenVersion &&
matchesConfigClass(req.configClassSet, versionedMutation.mutation.getConfigClass())) {
TraceEvent(SevDebug, "ConfigBroadcasterSendingChangeMutation", impl->id)
.detail("Version", versionedMutation.version)
.detail("ReqLastSeenVersion", req.lastSeenVersion)
.detail("ConfigClass", versionedMutation.mutation.getConfigClass())
.detail("KnobName", versionedMutation.mutation.getKnobName())
.detail("KnobValue", versionedMutation.mutation.getValue());
reply.versionedMutations.push_back_deep(reply.versionedMutations.arena(),
versionedMutation);
if (req.lastSeenVersion < impl->mostRecentVersion) {
impl->sendChangesReply(req, impl->versionedMutations);
} else {
auto token = req.reply.getEndpoint().token;
impl->tokenToRequest[token] = req;
ASSERT(req.configClassSet.present());
for (const auto& configClass : req.configClassSet.get().getClasses()) {
impl->configClassToTokens[configClass].push_back(token);
}
}
req.reply.send(reply);
++impl->successfulChangeRequest;
}
when(ConfigFollowerCompactRequest req = waitNext(cfi.compact.getFuture())) {
++impl->compactRequest;
// TODO: Use std::algorithm here
while (!impl->versionedMutations.empty()) {
const auto& version = impl->versionedMutations.front().version;
if (version > req.version) {
@ -121,19 +131,35 @@ class ConfigBroadcasterImpl {
public:
Future<Void> serve(ConfigBroadcaster* self, ConfigFollowerInterface const& cfi) { return serve(self, this, cfi); }
Future<Void> addVersionedMutations(Standalone<VectorRef<VersionedConfigMutationRef>> const& versionedMutations,
Future<Void> addVersionedMutations(Standalone<VectorRef<VersionedConfigMutationRef>> const& changes,
Version mostRecentVersion) {
this->versionedMutations.insert(
this->versionedMutations.end(), versionedMutations.begin(), versionedMutations.end());
for (const auto& versionedMutation : versionedMutations) {
this->mostRecentVersion = mostRecentVersion;
versionedMutations.insert(versionedMutations.end(), changes.begin(), changes.end());
std::set<Endpoint::Token> toNotify;
for (const auto& versionedMutation : changes) {
const auto& mutation = versionedMutation.mutation;
if (!mutation.getConfigClass().present()) {
// Update everything
for (const auto& [token, req] : tokenToRequest) {
toNotify.insert(token);
}
} else {
for (const auto& token : configClassToTokens[mutation.getConfigClass().get()]) {
toNotify.insert(token);
}
configClassToTokens.clear();
}
if (mutation.isSet()) {
snapshot[mutation.getKey()] = mutation.getValue().get();
} else {
snapshot.erase(mutation.getKey());
}
}
this->mostRecentVersion = mostRecentVersion;
for (const auto& token : toNotify) {
// TODO: What if this reply gets lost?
sendChangesReply(tokenToRequest[token], changes);
tokenToRequest.erase(token);
}
return Void();
}

View File

@ -56,3 +56,7 @@ ConfigClassSet::ConfigClassSet(VectorRef<KeyRef> configClasses) {
bool ConfigClassSet::contains(KeyRef configClass) const {
return classes.count(configClass);
}
std::set<Key> const& ConfigClassSet::getClasses() const {
return classes;
}

View File

@ -37,6 +37,7 @@ public:
ConfigClassSet(VectorRef<KeyRef> configClasses);
bool contains(KeyRef configClass) const;
std::set<Key> const& getClasses() const;
template <class Ar>
void serialize(Ar& ar) {

View File

@ -58,7 +58,8 @@ class ConfigKnobOverrides {
public:
ConfigKnobOverrides() = default;
explicit ConfigKnobOverrides(std::string const& paramString) {
// TODO: Validate string
ASSERT(std::all_of(
paramString.begin(), paramString.end(), [](char c) { return isalpha(c) || c == '/' || c == '-'; }));
StringRef s = stringToKeyRef(paramString);
while (s.size()) {
configPath.push_back_deep(configPath.arena(), s.eat("/"_sr));

View File

@ -53,7 +53,6 @@ public:
ClientKnobs const& getClientKnobs() const;
ServerKnobs const& getServerKnobs() const;
TestKnobs const& getTestKnobs() const;
// TODO: Only one field of serverDBInfo is required, so improve encapsulation
Future<Void> consume(Reference<IDependentAsyncVar<ConfigFollowerInterface> const> const& broadcaster);
Future<Void> setSnapshot(std::map<ConfigKey, Value> const& snapshot, Version snapshotVersion);
Future<Void> addVersionedMutations(Standalone<VectorRef<VersionedConfigMutationRef>> versionedMutations,

View File

@ -146,8 +146,7 @@ public:
template <class ConfigStore>
Future<Void> consume(ConfigStore& configStore) {
// TODO: Reenable compaction
return fetchChanges(this, &configStore); /* ||compactor(this); */
return fetchChanges(this, &configStore) || compactor(this);
}
UID getID() const { return id; }

View File

@ -30,11 +30,11 @@
namespace {
const KeyRef lastCompactedVersionKey = LiteralStringRef("lastCompactedVersion");
const KeyRef liveTransactionVersionKey = LiteralStringRef("liveTransactionVersion");
const KeyRef committedVersionKey = LiteralStringRef("committedVersion");
const KeyRangeRef kvKeys = KeyRangeRef(LiteralStringRef("kv/"), LiteralStringRef("kv0"));
const KeyRangeRef mutationKeys = KeyRangeRef(LiteralStringRef("mutation/"), LiteralStringRef("mutation0"));
const KeyRef lastCompactedVersionKey = "lastCompactedVersion"_sr;
const KeyRef liveTransactionVersionKey = "liveTransactionVersion"_sr;
const KeyRef committedVersionKey = "committedVersion"_sr;
const KeyRangeRef kvKeys = KeyRangeRef("kv/"_sr, "kv0"_sr);
const KeyRangeRef mutationKeys = KeyRangeRef("mutation/"_sr, "mutation0"_sr);
Key versionedMutationKey(Version version, uint32_t index) {
ASSERT(version >= 0);