Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22710 )
Change subject: [tablet] fix result status handling ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/22710/3/src/kudu/tablet/tablet_bootstrap-test.cc File src/kudu/tablet/tablet_bootstrap-test.cc: http://gerrit.cloudera.org:8080/#/c/22710/3/src/kudu/tablet/tablet_bootstrap-test.cc@253 PS3, Line 253: SCOPED_TRACE(Substitute("entry $0", i)); > why is this necessary here? If the assertion ever triggers, it's helpful to understand what entry that was. http://gerrit.cloudera.org:8080/#/c/22710/3/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/22710/3/src/kudu/tablet/tablet_replica.cc@656 PS3, Line 656: void TabletReplica::RunLogGC() { > Could you explain the reasoning behind this change? As you could see, RunLogGC() has never returned non-OK status: even before this patch, all it did was to output an error message into the log, making it essentially 'void' function. So, this update hasn't changed anything in this regard. I checked the history of the code around: there was some refactoring in 4d66bf7c2 that changed the return type of this method from 'void' to 'Status', but it didn't change the semantics of this method of just loggin on errors, as it had been before. -- To view, visit http://gerrit.cloudera.org:8080/22710 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id5407906f2bf65e9a8798233f0280591e6e975cf Gerrit-Change-Number: 22710 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Mon, 31 Mar 2025 16:11:35 +0000 Gerrit-HasComments: Yes
