This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push: new ec8c31d36 [catalog_manager] Tighten leader UUID fallback ec8c31d36 is described below commit ec8c31d36f1bb2bf634565e0bbae43479f74deef Author: Ádám Bakai <aba...@cloudera.com> AuthorDate: Tue Feb 6 11:40:31 2024 +0100 [catalog_manager] Tighten leader UUID fallback It is safe to assume that if the term is the same in the current cstate as in the previous cstate then even if the leader is not set, it will be the same. But it is possible that cmeta file is deleted then recreated with "local_replica cmeta unsafe_recreate" command. In this case the leader_uuid is empty in the new cmeta file. This means that the peer doesn't consider itself a leader, so no health report is generated in tablet report and it has no leader_uuid set either. When a master receives tablet report like this and there isn't a new term, then the catalog master will treat this peer as a leader, but it will fail on a check because the leader has to be in healthy status. This happened in ToolTest::TestRecreateCMeta. As a reproduction step, the same test now runs with a single TServer configuration, too. In this configuration the error is reproducible 100% of the times, since the term is not increased and the leader's cmeta file is changed. The solution is that catalog manager only assumes the previous leader for the peer if the previous leader is not the peer itself. This gives time for the peers to form a consensus about the leader. Change-Id: I06a80a4a0a9fd422b50860e8cd8bf0e12973cd43 Reviewed-on: http://gerrit.cloudera.org:8080/21004 Tested-by: Alexey Serbin <ale...@apache.org> Reviewed-by: Alexey Serbin <ale...@apache.org> --- src/kudu/master/catalog_manager.cc | 12 ++++++++++-- src/kudu/tools/kudu-tool-test.cc | 11 +++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc index 01acfcdfe..891ef0320 100644 --- a/src/kudu/master/catalog_manager.cc +++ b/src/kudu/master/catalog_manager.cc @@ -5453,9 +5453,17 @@ Status CatalogManager::ProcessTabletReport( // the leader because the follower doing the reporting may not know who // the leader is yet (it may have just started up). It is safe to reuse // the previous leader if the reported cstate has the same term as the - // previous cstate, and the leader was known for that term. + // previous cstate, and the leader was known for that term. An extra + // condition is to check whether it's a report from a former leader + // replica which currently doesn't maintain the leadership role. Such a + // situation is possible when the replica's cmeta file had been deleted + // and then recreated (e.g. by the "kudu local_replica cmeta unsafe_recreate" + // CLI tool). The code below assumes that the replica effectively has + // the leadership if the 'leader_uuid' field is set, but that's not so + // (see KUDU-2335). if (cstate.current_term() == prev_cstate.current_term()) { - if (cstate.leader_uuid().empty() && !prev_cstate.leader_uuid().empty()) { + if (cstate.leader_uuid().empty() && !prev_cstate.leader_uuid().empty() && + ts_desc->permanent_uuid() != prev_cstate.leader_uuid()) { cstate.set_leader_uuid(prev_cstate.leader_uuid()); // Sanity check to detect consensus divergence bugs. } else if (!cstate.leader_uuid().empty() && diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc index ee93845de..5dd5487c8 100644 --- a/src/kudu/tools/kudu-tool-test.cc +++ b/src/kudu/tools/kudu-tool-test.cc @@ -8503,9 +8503,16 @@ TEST_F(ToolTest, TestCheckFSWithNonDefaultMetadataDir) { SCOPED_TRACE(stdout); } -TEST_F(ToolTest, TestRecreateCMeta) { +class RecreateCMetaTest : + public ToolTest, + public ::testing::WithParamInterface<bool> { +}; + +INSTANTIATE_TEST_SUITE_P(, RecreateCMetaTest, ::testing::Bool()); +TEST_P(RecreateCMetaTest, TestRecreateCMeta) { SKIP_IF_SLOW_NOT_ALLOWED(); - constexpr int kNumTservers = 3; + const bool singleTserver = GetParam(); + const int kNumTservers = singleTserver ? 1 : 3; constexpr int kNumTablets = 1; constexpr int kNumRows = 1000; const MonoDelta kTimeout = MonoDelta::FromSeconds(30);