Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8606 )
Change subject: tablet: add early returns to maintenance functions ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/8606/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8606/5//COMMIT_MSG@11 PS5, Line 11: prevent > This sounds like reasonable approach. Why bother with trying to unschedule Mentioned in slack, this only aborts execution, but they can still be scheduled. If that happens, we can get stuck in a loop of scheduling an op and aborting (unless the op is eventually canceled). http://gerrit.cloudera.org:8080/#/c/8606/5/src/kudu/integration-tests/stop_tablet-itest.cc File src/kudu/integration-tests/stop_tablet-itest.cc: http://gerrit.cloudera.org:8080/#/c/8606/5/src/kudu/integration-tests/stop_tablet-itest.cc@93 PS5, Line 93: hog the maintenance threads > Shouldn't the "prepare" step of the MM ops simply check for kOpen state as I think that's an optimization, but it wouldn't get the same result as canceling ops, since the ops would still be scheduled. http://gerrit.cloudera.org:8080/#/c/8606/5/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/8606/5/src/kudu/tablet/tablet_replica.cc@690 PS5, Line 690: state_change_guard > nit: there are lots of variable names to read here already; we usually just Done http://gerrit.cloudera.org:8080/#/c/8606/5/src/kudu/tablet/tablet_replica_mm_ops.cc File src/kudu/tablet/tablet_replica_mm_ops.cc: http://gerrit.cloudera.org:8080/#/c/8606/5/src/kudu/tablet/tablet_replica_mm_ops.cc@200 PS5, Line 200: LOG(WARNING) << Substitute("$0 failed to flush DMS: $1", > nit: when logging, it's best to use the standard prefix when possible, whic Done -- To view, visit http://gerrit.cloudera.org:8080/8606 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I84ad557851863f6fd9acff28ddcd1244e62cf516 Gerrit-Change-Number: 8606 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 21 Nov 2017 09:25:46 +0000 Gerrit-HasComments: Yes
