From d398377c4a98796cae4fd0d8c4f19d39c61c9f6f Mon Sep 17 00:00:00 2001 From: Sreenath Bodagala Date: Mon, 26 Jul 2021 16:27:19 +0000 Subject: [PATCH 1/4] - Move the logic that populates written_tags (the tags that have been affected by a transaction batch) to LogPushData --- fdbclient/FDBTypes.h | 2 ++ fdbserver/CommitProxyServer.actor.cpp | 3 +-- fdbserver/LogSystem.h | 11 +++++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/fdbclient/FDBTypes.h b/fdbclient/FDBTypes.h index 741b4c2c81..3db1527733 100644 --- a/fdbclient/FDBTypes.h +++ b/fdbclient/FDBTypes.h @@ -74,6 +74,8 @@ struct Tag { int toTagDataIndex() const { return locality >= 0 ? 2 * locality : 1 - (2 * locality); } + bool isPseudoTag() const { return locality < 0; } + std::string toString() const { return format("%d:%d", locality, id); } template diff --git a/fdbserver/CommitProxyServer.actor.cpp b/fdbserver/CommitProxyServer.actor.cpp index c3b66a13d5..68f46b4f23 100644 --- a/fdbserver/CommitProxyServer.actor.cpp +++ b/fdbserver/CommitProxyServer.actor.cpp @@ -965,7 +965,6 @@ ACTOR Future assignMutationsToStorageServers(CommitBatchContext* self) { if (pProxyCommitData->cacheInfo[m.param1]) { self->toCommit.addTag(cacheTag); } - self->toCommit.saveTags(self->writtenTags); self->toCommit.writeTypedMessage(m); self->toCommit.saveLocations(self->writtenTLogs); } else if (m.type == MutationRef::ClearRange) { @@ -1023,7 +1022,6 @@ ACTOR Future assignMutationsToStorageServers(CommitBatchContext* self) { if (pProxyCommitData->needsCacheTag(clearRange)) { self->toCommit.addTag(cacheTag); } - self->toCommit.saveTags(self->writtenTags); self->toCommit.writeTypedMessage(m); self->toCommit.saveLocations(self->writtenTLogs); } else { @@ -1212,6 +1210,7 @@ ACTOR Future postResolution(CommitBatchContext* self) { if (SERVER_KNOBS->ENABLE_VERSION_VECTOR) { tpcvMap = self->tpcvMap; } + self->toCommit.saveTags(self->writtenTags); self->loggingComplete = pProxyCommitData->logSystem->push(self->prevVersion, self->commitVersion, pProxyCommitData->committedVersion.get(), diff --git a/fdbserver/LogSystem.h b/fdbserver/LogSystem.h index 8d979f80a9..4fd713c542 100644 --- a/fdbserver/LogSystem.h +++ b/fdbserver/LogSystem.h @@ -997,8 +997,12 @@ struct LogPushData : NonCopyable { } // copy next_message_tags into given set - void saveTags(std::set& writtenTags) { - writtenTags.insert(next_message_tags.begin(), next_message_tags.end()); + void saveTags(std::set& writtenTags, bool filterPseudoTags = true) { + for (auto& tag : written_tags) { + if (!filterPseudoTags || !tag.isPseudoTag()) { + writtenTags.insert(tag); + } + } } // store tlogs as represented by index @@ -1017,6 +1021,7 @@ struct LogPushData : NonCopyable { } msg_locations.clear(); logSystem->getPushLocations(prev_tags, msg_locations); + written_tags.insert(written_tags.end(), next_message_tags.begin(), next_message_tags.end()); next_message_tags.clear(); } uint32_t subseq = this->subsequence++; @@ -1095,6 +1100,7 @@ struct LogPushData : NonCopyable { wr.serializeBytes((uint8_t*)from.getData() + firstOffset, firstLength); } } + written_tags.insert(written_tags.end(), next_message_tags.begin(), next_message_tags.end()); next_message_tags.clear(); } @@ -1104,6 +1110,7 @@ private: Reference logSystem; std::vector next_message_tags; std::vector prev_tags; + std::vector written_tags; std::vector messagesWriter; std::vector msg_locations; // Stores message locations that have had span information written to them From de020efa7fe21394626a22f1e95f81045b8a2e6b Mon Sep 17 00:00:00 2001 From: Sreenath Bodagala Date: Mon, 26 Jul 2021 17:13:31 +0000 Subject: [PATCH 2/4] - Correct a comment --- fdbserver/LogSystem.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdbserver/LogSystem.h b/fdbserver/LogSystem.h index 4fd713c542..c5836980a2 100644 --- a/fdbserver/LogSystem.h +++ b/fdbserver/LogSystem.h @@ -996,7 +996,7 @@ struct LogPushData : NonCopyable { writtenLocations.clear(); } - // copy next_message_tags into given set + // copy written_tags into given set void saveTags(std::set& writtenTags, bool filterPseudoTags = true) { for (auto& tag : written_tags) { if (!filterPseudoTags || !tag.isPseudoTag()) { From 82774ae62c7fc147ce43196ce4094884e219ec44 Mon Sep 17 00:00:00 2001 From: Sreenath Bodagala Date: Mon, 26 Jul 2021 19:39:08 +0000 Subject: [PATCH 3/4] - Address review comments --- fdbclient/FDBTypes.h | 2 +- fdbserver/LogSystem.h | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/fdbclient/FDBTypes.h b/fdbclient/FDBTypes.h index 3db1527733..a60b8ca743 100644 --- a/fdbclient/FDBTypes.h +++ b/fdbclient/FDBTypes.h @@ -74,7 +74,7 @@ struct Tag { int toTagDataIndex() const { return locality >= 0 ? 2 * locality : 1 - (2 * locality); } - bool isPseudoTag() const { return locality < 0; } + bool isNonPrimaryTLogType() const { return locality < 0; } std::string toString() const { return format("%d:%d", locality, id); } diff --git a/fdbserver/LogSystem.h b/fdbserver/LogSystem.h index c5836980a2..96c9d32462 100644 --- a/fdbserver/LogSystem.h +++ b/fdbserver/LogSystem.h @@ -996,11 +996,11 @@ struct LogPushData : NonCopyable { writtenLocations.clear(); } - // copy written_tags into given set - void saveTags(std::set& writtenTags, bool filterPseudoTags = true) { - for (auto& tag : written_tags) { - if (!filterPseudoTags || !tag.isPseudoTag()) { - writtenTags.insert(tag); + // copy written_tags, after filtering, into given set + void saveTags(std::set& filteredTags) const { + for (const auto& tag : written_tags) { + if (!tag.isNonPrimaryTLogType()) { + filteredTags.insert(tag); } } } @@ -1021,7 +1021,7 @@ struct LogPushData : NonCopyable { } msg_locations.clear(); logSystem->getPushLocations(prev_tags, msg_locations); - written_tags.insert(written_tags.end(), next_message_tags.begin(), next_message_tags.end()); + written_tags.insert(next_message_tags.begin(), next_message_tags.end()); next_message_tags.clear(); } uint32_t subseq = this->subsequence++; @@ -1100,7 +1100,7 @@ struct LogPushData : NonCopyable { wr.serializeBytes((uint8_t*)from.getData() + firstOffset, firstLength); } } - written_tags.insert(written_tags.end(), next_message_tags.begin(), next_message_tags.end()); + written_tags.insert(next_message_tags.begin(), next_message_tags.end()); next_message_tags.clear(); } @@ -1110,7 +1110,7 @@ private: Reference logSystem; std::vector next_message_tags; std::vector prev_tags; - std::vector written_tags; + std::set written_tags; std::vector messagesWriter; std::vector msg_locations; // Stores message locations that have had span information written to them From 7f3bc538584da7951160e10d1c0edc6a6a2b17d0 Mon Sep 17 00:00:00 2001 From: Sreenath Bodagala Date: Mon, 2 Aug 2021 19:35:59 +0000 Subject: [PATCH 4/4] - Address a bug --- fdbserver/CommitProxyServer.actor.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fdbserver/CommitProxyServer.actor.cpp b/fdbserver/CommitProxyServer.actor.cpp index 68f46b4f23..862ab8b427 100644 --- a/fdbserver/CommitProxyServer.actor.cpp +++ b/fdbserver/CommitProxyServer.actor.cpp @@ -1113,6 +1113,8 @@ ACTOR Future postResolution(CommitBatchContext* self) { // Second pass wait(assignMutationsToStorageServers(self)); + self->toCommit.saveTags(self->writtenTags); + // Obtain previous committed versions for each affected tlog from sequencer if (SERVER_KNOBS->ENABLE_VERSION_VECTOR) { wait(getTPCV(self)); @@ -1210,7 +1212,6 @@ ACTOR Future postResolution(CommitBatchContext* self) { if (SERVER_KNOBS->ENABLE_VERSION_VECTOR) { tpcvMap = self->tpcvMap; } - self->toCommit.saveTags(self->writtenTags); self->loggingComplete = pProxyCommitData->logSystem->push(self->prevVersion, self->commitVersion, pProxyCommitData->committedVersion.get(),