Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8606 )
Change subject: tablet: add early returns to maintenance functions ...................................................................... Patch Set 5: (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 them all when we can just do this? 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 well, then? 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 name these guards "l". At first scan I thought we managed to name a lock "state_change_guard" 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("Failed to flush DMS on $0: $1", nit: when logging, it's best to use the standard prefix when possible, which is T <tablet id> P <server uuid>: Message Because it allows for easily grepping through large archives of log files. -- 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: 5 Gerrit-Owner: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Tue, 21 Nov 2017 05:06:53 +0000 Gerrit-HasComments: Yes
