Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16526 )
Change subject: KUDU-2612 p5 (b): add highest_seen_txn_id into CoordinatorOpResultPB ...................................................................... Patch Set 4: Code-Review+1 (1 comment) Will leave this open since I think Hao should also review the pieces that are relevant to her tablet-loading work. http://gerrit.cloudera.org:8080/#/c/16526/3/src/kudu/transactions/txn_status_manager.cc File src/kudu/transactions/txn_status_manager.cc: http://gerrit.cloudera.org:8080/#/c/16526/3/src/kudu/transactions/txn_status_manager.cc@164 PS3, Line 164: // Always set the 'highest_seen_txn_id' on return. : SCOPED_CLEANUP({ : if (highest_seen_txn_id) { : std::lock_guard<simple_spinlock> l(lock_); : *highest_seen_txn_id = highest_txn_id_; : } : }); > I was considering that as the very first approach, but I'd like to have 'hi OK sounds reasonable, particularly with concurrent activity. My main concern is that exposing the transaction IDs on all errors might open the door for relying on the -2 and -1 magic numbers instead of the 'ts_error' or return status. I'd rather not have anything rely on such magic numbers because I expect the approach to change near-term since Hao is working on loading the tablet upon leadership change, in which case the "loaded" state might be tracked separately. That said, that sort of thing should be easily caught via code review, and also if we treat the 'highest_seen_txn_id' as undefined for all non-InvalidArgument errors. -- To view, visit http://gerrit.cloudera.org:8080/16526 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcb4d90bc10a5695c3f54229688ccdcaf56011d0 Gerrit-Change-Number: 16526 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Sat, 03 Oct 2020 01:39:59 +0000 Gerrit-HasComments: Yes
