Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24287 )
Change subject: KUDU-3762 - Fix catchup deadlock on LMP mismatch ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/24287/3/src/kudu/consensus/consensus_queue.cc File src/kudu/consensus/consensus_queue.cc: http://gerrit.cloudera.org:8080/#/c/24287/3/src/kudu/consensus/consensus_queue.cc@1050 PS3, Line 1050: LookupOpId Is it possible to do this outside queue_lock_? This does a read on index file (in the absence of page in log cache) that could end up in stalled state while holding queue_lock_, blocking other consensus queue operations: LookupOpId -> log_->reader()->LookupOpId -> log_index_->GetEntry -> chunk->GetEntry -> file_->Read http://gerrit.cloudera.org:8080/#/c/24287/3/src/kudu/consensus/consensus_queue.cc@1054 PS3, Line 1054: The peer is actually AHEAD of the cache > That's the other (remote) peer is ahead of the cache of this (local) peer, I think it is not just cache but the leader's own log index tip. Maybe both mean the same in this context but it wouldn't harm to state the same clearly here. ++ // We sometimes try to look up OpIds that have never been written // on the local node. In that case, don't try to read the op from // the log reader, since it might actually race against the writing // of the op. if (op_index >= next_sequential_op_index_) { return Status::Incomplete(Substitute("Op with index $0 is ahead of the local log "(next sequential op: $1)", op_index, next_sequential_op_index_)); } +++ http://gerrit.cloudera.org:8080/#/c/24287/3/src/kudu/mini-cluster/external_mini_cluster-test.cc File src/kudu/mini-cluster/external_mini_cluster-test.cc: http://gerrit.cloudera.org:8080/#/c/24287/3/src/kudu/mini-cluster/external_mini_cluster-test.cc@463 PS3, Line 463: ASSERT_OK(table_creator->table_name(table_name) : .schema(&schema) : .num_replicas(3) : .add_hash_partitions({"key"}, 16) : .Create()); : ASSERT_EVENTUALLY([&] { : bool in_progress = true; : ASSERT_OK(new_client->IsCreateTableInProgress(table_name, &in_progress)); : ASSERT_FALSE(in_progress); : }); : ASSERT_OK(new_client->DeleteTable(table_name)) If any of these fail during an iteration, I believe churn thread will continue with the loop without letting main thread to detect a failure. I had hit something similar in one of my patches. Please verify if you expect this to stop when an assertion fails. -- To view, visit http://gerrit.cloudera.org:8080/24287 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00f4f840c21cc037a2aa6c023af168352fef0761 Gerrit-Change-Number: 24287 Gerrit-PatchSet: 3 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Tue, 12 May 2026 07:18:52 +0000 Gerrit-HasComments: Yes
