Skip to content

[Bugfix][Store] Keep scanning for a local MEMORY replica past a local NoF one #3742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
2sumtech wants to merge 1 commit into from
Closed

[Bugfix][Store] Keep scanning for a local MEMORY replica past a local NoF one #3742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion mooncake-store/include/replica_selection.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ inline const Replica::Descriptor *SelectBestReplica(
const std::unordered_set<std::string> &local_endpoints) {
const Replica::Descriptor *first_memory = nullptr;
const Replica::Descriptor *first_nof = nullptr;
const Replica::Descriptor *local_nof = nullptr;
for (const auto &r : replicas) {
if (r.status != ReplicaStatus::COMPLETE) continue;
if (r.is_memory_replica()) {
Expand All @@ -138,11 +139,16 @@ inline const Replica::Descriptor *SelectBestReplica(
if (local_endpoints.count(
r.get_nof_descriptor()
.buffer_descriptor.transport_endpoint_)) {
return &r; // local NOF_SSD — also good
// local NOF_SSD — good, but local MEMORY outranks it and may
// still appear later, so remember it and keep scanning.
if (!local_nof) local_nof = &r;
continue;
}
if (!first_nof) first_nof = &r;
}
}
// No local MEMORY replica: a local NOF_SSD still beats every remote one.
if (local_nof) return local_nof;
// No local replica. Among remote MEMORY replicas, optionally pick the
// best-scoring one instead of the first encountered (issue #2516).
if (first_memory && RemoteReplicaScoringEnabled()) {
Expand Down
38 changes: 38 additions & 0 deletions mooncake-store/tests/replica_selection_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,44 @@ TEST_F(ReplicaSelectionTest, LocalMemoryAlwaysWins) {
"nodeB"); // locality beats protocol
}

TEST_F(ReplicaSelectionTest, LocalMemoryWinsOverEarlierLocalNoF) {
// Master may return replicas in any order, so a local NOF_SSD listed
// before a local MEMORY replica must not win: local MEMORY outranks it.
std::unordered_set<std::string> local = {"nodeA"};
std::vector<Replica::Descriptor> reps = {
MakeNoF("nodeA"), // local, but slower tier
MakeMemory("nodeA", "rdma"), // local MEMORY -> must win
};
const auto* sel = SelectBestReplica(reps, local);
ASSERT_NE(sel, nullptr);
EXPECT_TRUE(sel->is_memory_replica());
}

TEST_F(ReplicaSelectionTest, IncompleteLocalMemoryFallsBackToLocalNoF) {
// The local MEMORY replica is not readable yet, so the local NOF_SSD one
// remains the best choice.
std::unordered_set<std::string> local = {"nodeA"};
std::vector<Replica::Descriptor> reps = {
MakeNoF("nodeA"),
MakeMemory("nodeA", "rdma", ReplicaStatus::PROCESSING),
};
const auto* sel = SelectBestReplica(reps, local);
ASSERT_NE(sel, nullptr);
EXPECT_TRUE(sel->is_nof_replica());
}

TEST_F(ReplicaSelectionTest, LocalNoFPrecedesRemoteMemory) {
// Locality still outranks tier for remote MEMORY replicas.
std::unordered_set<std::string> local = {"nodeA"};
std::vector<Replica::Descriptor> reps = {
MakeNoF("nodeA"),
MakeMemory("nodeB", "rdma"),
};
const auto* sel = SelectBestReplica(reps, local);
ASSERT_NE(sel, nullptr);
EXPECT_TRUE(sel->is_nof_replica());
}

TEST_F(ReplicaSelectionTest, ScoringOffKeepsFirstRemoteMemory) {
// No scorer injected and env not set -> must return the FIRST remote
// MEMORY.
Expand Down
Loading