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

Reply via email to