Skip to content

Commit

Permalink
ClientDBInfo may be unintentionally not set
Browse files Browse the repository at this point in the history
The ClientDBInfo's comparison is through an internal UID and shrinkProxyList()
can change proxies inside ClientDBInfo. Since the UID is not changed by that
function, subsequent set can be unintentionally skipped.

This was not a big issue before. However, VV introduces a change that the
client side compares the returned proxy ID with its known set of GRV proxies
and will retry GRV if the returned proxy ID is not in the set. Due the above
bug, GRV returned by a proxy is not within the client set, and results in
indefinite retrying GRVs.
  • Loading branch information
jzhou77 committed Apr 16, 2022
1 parent 6151f9c commit 086f747
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion fdbclient/MonitorLeader.actor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,7 @@ ACTOR Future<MonitorLeaderInfo> monitorProxiesOneGeneration(

auto& ni = rep.get().mutate();
shrinkProxyList(ni, lastCommitProxyUIDs, lastCommitProxies, lastGrvProxyUIDs, lastGrvProxies);
clientInfo->set(ni);
clientInfo->setUnconditional(ni);
successIndex = index;
} else {
TEST(rep.getError().code() == error_code_failed_to_progress); // Coordinator cant talk to cluster controller
Expand Down
2 changes: 1 addition & 1 deletion fdbserver/worker.actor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ ACTOR static Future<Void> extractClientInfo(Reference<AsyncVar<ServerDBInfo> con
loop {
ClientDBInfo ni = db->get().client;
shrinkProxyList(ni, lastCommitProxyUIDs, lastCommitProxies, lastGrvProxyUIDs, lastGrvProxies);
info->set(ni);
info->setUnconditional(ni);
wait(db->onChange());
}
}
Expand Down

0 comments on commit 086f747

Please sign in to comment.