Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/17159 )
Change subject: KUDU-2612: acquire and release partition lock ...................................................................... Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/17159/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17159/1//COMMIT_MSG@13 PS1, Line 13: COMMI > We'll also need to make sure the TxnOpDispatcher handles the error cases we Done http://gerrit.cloudera.org:8080/#/c/17159/1/src/kudu/tablet/ops/participant_op.cc File src/kudu/tablet/ops/participant_op.cc: http://gerrit.cloudera.org:8080/#/c/17159/1/src/kudu/tablet/ops/participant_op.cc@95 PS1, Line 95: l acquired = partition_lock_.IsAcquired(&code); > nit: based on the current code, this cannot be any other code, right? If so Done http://gerrit.cloudera.org:8080/#/c/17159/1/src/kudu/tablet/ops/participant_op.cc@199 PS1, Line 199: "ParticipantOp::Prepare > nit: can use 'replica' Done http://gerrit.cloudera.org:8080/#/c/17159/1/src/kudu/tablet/ops/participant_op.cc@200 PS1, Line 200: TRACE("PREPARE: Starting."); > Shouldn't there be a 'break' before this line? Or are we taking the partiti Ah, right, 'break' is needed. Thanks for catching it! http://gerrit.cloudera.org:8080/#/c/17159/1/src/kudu/tablet/ops/participant_op.cc@300 PS1, Line 300: return Status::InvalidArgument("unknown op type"); > As for the sequence of releasing txn and its lock and partition lock: shoul Yes, it makes sense. Updated it. http://gerrit.cloudera.org:8080/#/c/17159/1/src/kudu/tablet/ops/participant_op.cc@303 PS1, Line 303: return Status::OK(); > nit: could store the type of the operation instead since the operation as i Will do. http://gerrit.cloudera.org:8080/#/c/17159/1/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: http://gerrit.cloudera.org:8080/#/c/17159/1/src/kudu/tablet/tablet.cc@223 PS1, Line 223: using std::endl; > warning: using decl 'TabletServerErrorPB' is unused [misc-unused-using-decl Done -- To view, visit http://gerrit.cloudera.org:8080/17159 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If26733cae16810f3b3afd1fd05dcb984e6366939 Gerrit-Change-Number: 17159 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 11 Mar 2021 18:57:01 +0000 Gerrit-HasComments: Yes
