Todd Lipcon has posted comments on this change. Change subject: Expose running maintenance op info ......................................................................
Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/7537/4/src/kudu/tserver/tserver-path-handlers.cc File src/kudu/tserver/tserver-path-handlers.cc: PS4, Line 629: op_pb.duration_millis() / 1000.0), : HumanReadableElapsedTime::ToShortString( : op_pb.millis_since_start())); why is one of these /1000 and the other not? http://gerrit.cloudera.org:8080/#/c/7537/3/src/kudu/util/maintenance_manager.h File src/kudu/util/maintenance_manager.h: Line 153: // Name of operation. > Added a sentinel value and a comment. MonoDelta's default constructor already puts it into an "uninitialized" state so you don't need a special sentinel here. You can just use .Initialized() to check if it has been initialized. Line 319: > This was actually preexisting, I just moved it down a couple lines. Do you ah, that's ok then http://gerrit.cloudera.org:8080/#/c/7537/4/src/kudu/util/maintenance_manager.h File src/kudu/util/maintenance_manager.h: PS4, Line 160: nit: indentation is off Line 162: pb->set_thread_id(ss.str()); ah, I see that std::to_string won't work here. But if this is an opaque number that isn't really useful to users, I think we'd be better off using Thread::current_thread()->name() -- 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: 4 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: Sam Okrent <samuel.okr...@cloudera.com> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
