Gabriella Lotz has posted comments on this change. ( http://gerrit.cloudera.org:8080/24637 )
Change subject: KUDU-3751: master steps down instead of crashing on catch-up timeout ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/24637/4/src/kudu/master/catalog_manager-test.cc File src/kudu/master/catalog_manager-test.cc: http://gerrit.cloudera.org:8080/#/c/24637/4/src/kudu/master/catalog_manager-test.cc@515 PS4, Line 515: SleepFor(MonoDelta::FromMilliseconds( : FLAGS_tablet_inject_latency_on_apply_write_op_ms + : FLAGS_master_failover_catchup_timeout_ms + 5000)); : } The fixed SleepFor(~14s) dominates the test's wall time. Do you think ASSERT_EVENTUALLY (e.g. on consensus->role()) would be a better fit here? http://gerrit.cloudera.org:8080/#/c/24637/4/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/24637/4/src/kudu/master/catalog_manager.cc@1453 PS4, Line 1453: if (step_down_resp.has_error()) { : LOG(WARNING) << "Step-down response error: " : << step_down_resp.error().status().message(); : } I'm not sure this warning is quite right. If I'm reading RaftConsensus::StepDown() correctly, it returns Status::OK and sets resp.error() to NOT_THE_LEADER when we're already not the leader, so this branch would fire in the benign race where someone else took over between the catch-up timeout and our StepDown call, logging what looks like an error but is actually the outcome we wanted. Would it make sense to treat NOT_THE_LEADER as success here, or drop this to LOG(INFO)? -- To view, visit http://gerrit.cloudera.org:8080/24637 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c9c5a3a6ec43c5174f8d8e92e2421029c51ea01 Gerrit-Change-Number: 24637 Gerrit-PatchSet: 4 Gerrit-Owner: Jeison Romero <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Jeison Romero <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Tue, 28 Jul 2026 15:34:04 +0000 Gerrit-HasComments: Yes
