Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22867 )
Change subject: KUDU-1973: Send no-op heartbeat operations batched - PART 1 ...................................................................... Patch Set 17: Code-Review+1 (3 comments) Overall looks good to me. I guess Marton and Attila might have some feedback as well, so adding +1 as of now. Unless I'm missing something, I'm ready to give it +2 once Marton and Attila find this looking good to them as well. Thank you for working on this! http://gerrit.cloudera.org:8080/#/c/22867/15/src/kudu/consensus/multi_raft_batcher.cc File src/kudu/consensus/multi_raft_batcher.cc: http://gerrit.cloudera.org:8080/#/c/22867/15/src/kudu/consensus/multi_raft_batcher.cc@275 PS15, Line 275: auto raft_pool_token = raft_pool_->NewToken(ThreadPool::ExecutionMode::CONCURRENT); : batcher = std::make_shared<MultiRaftHeartbeatBatcher>( : hostport, dns_resolver_, messenger_, batch_time_window_, move(raft_pool_token)); : batcher->StartTimer(); : batchers_[hostport] = batcher; : return batcher; : } : : void MultiRaftManager::Shutdown() { : s > I change it to have a raft pool token for each batcher. Since it is in con That's a better option, indeed. Thanks! http://gerrit.cloudera.org:8080/#/c/22867/17/src/kudu/consensus/multi_raft_consensus_data.h File src/kudu/consensus/multi_raft_consensus_data.h: http://gerrit.cloudera.org:8080/#/c/22867/17/src/kudu/consensus/multi_raft_consensus_data.h@63 PS17, Line 63: if (req.has_preceding_id()) : { style nit: fix the curly bracket placement http://gerrit.cloudera.org:8080/#/c/22867/15/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/22867/15/src/kudu/tserver/tablet_service.cc@1830 PS15, Line 1830: } > it is returned by value: OK, but 'const auto&' would work here as well. The main point was to have 'const' to let readers know 'stat' isn't changing. That was a nit, anyway. -- To view, visit http://gerrit.cloudera.org:8080/22867 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie92ba4de5eae00d56cd513cb644dce8fb6e14538 Gerrit-Change-Number: 22867 Gerrit-PatchSet: 17 Gerrit-Owner: Zoltan Martonka <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Mon, 22 Sep 2025 05:58:30 +0000 Gerrit-HasComments: Yes
