Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20300 )
Change subject: KUDU-3500 don't start operations timed out in prepare queue ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/20300/2/src/kudu/tablet/ops/write_op.cc File src/kudu/tablet/ops/write_op.cc: http://gerrit.cloudera.org:8080/#/c/20300/2/src/kudu/tablet/ops/write_op.cc@76 PS2, Line 76: ; > nit: Maybe use .(fullstop) to be consistent with tablet_inject_latency_on_a Done http://gerrit.cloudera.org:8080/#/c/20300/2/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/20300/2/src/kudu/tserver/tablet_service.cc@1681 PS2, Line 1681: deadline > What are your thoughts on storing this deadline in a member variable of kud I though about that before opting out for the current approach implemented in the patch and I didn't see the how the deadline fits OpsState with the existing types of operations. We have only WriteOpState where it makes sense, and the rest are AlterSchemaOpState, ParticipantOpState where the timeout doesn't quite fit. By my understanding, even in the case of replicated write operation the timeout isn't exactly fit to be a field/member of the WriteOpState. Also, I don't believe in the provisions of something for hypothetical future extensions: usually, those "extensions" never happen, or when they happen, you realize the majority of the prep work should be re-done. That's because it's much easier to write the better code once you know exactly what's needed. -- To view, visit http://gerrit.cloudera.org:8080/20300 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I202ce6b5e425439b50c0751d7f7406e69b8e751a Gerrit-Change-Number: 20300 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Thu, 03 Aug 2023 01:18:39 +0000 Gerrit-HasComments: Yes
