Andrew Wong has posted comments on this change. Change subject: Expose running maintenance op info ......................................................................
Patch Set 2: (10 comments) http://gerrit.cloudera.org:8080/#/c/7537/2/src/kudu/tserver/tserver-path-handlers.h File src/kudu/tserver/tserver-path-handlers.h: PS2, Line 26: #include "kudu/util/easy_json.h" Should be able to forward declare this. http://gerrit.cloudera.org:8080/#/c/7537/2/src/kudu/util/maintenance_manager-test.cc File src/kudu/util/maintenance_manager-test.cc: PS2, Line 314: manager_->UnregisterOp(&op1); Why is this necessary? http://gerrit.cloudera.org:8080/#/c/7537/2/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: PS2, Line 513: vector<OpInstance> running_instances; : op->GetRunningInstances(&running_instances); May be easier in this case to return the vector directly. http://gerrit.cloudera.org:8080/#/c/7537/2/src/kudu/util/maintenance_manager.h File src/kudu/util/maintenance_manager.h: PS2, Line 149: Holds the information regarding a recently completed operation. Should this comment be updated? PS2, Line 207: Note: op_instance is owned by the caller Ah, I see. In that case, can you also make this note down by the definition of running_instances_? Also, is there an expected caller besides the MaintenanceManager? PS2, Line 208: std::thread::id thread_id nit: const ref here and below? PS2, Line 214: OpInstance* Where is this output used? PS2, Line 216: instances nit: out_instances PS2, Line 245: nstances of this op Who owns the instances? Are there any lifetime guarantees we need to know about? PS2, Line 245: t they're ru nit: typo -- To view, visit http://gerrit.cloudera.org:8080/7537 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ide228d7e70deae3ae89d108cbd270f3f0f2580ca Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sam Okrent <samuel.okr...@cloudera.com> Gerrit-Reviewer: Andrew Wong <andrew.w...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes