Henry Robinson has posted comments on this change.

Change subject: IMPALA-4885: Expose Jvm thread info in web UI
......................................................................


Patch Set 3: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6013/3/be/src/util/thread.cc
File be/src/util/thread.cc:

PS3, Line 79: args
long line


PS3, Line 340: void InitThreading() {
             :   DCHECK(thread_manager.get() == nullptr);
             :   thread_manager.reset(new ThreadMgr());
             : }
             : 
             : Status StartThreadInstrumentation(MetricGroup* metrics, 
Webserver* webserver,
             :     bool include_jvm_threads) {
             :   DCHECK(metrics != nullptr);
             :   DCHECK(webserver != nullptr);
             :   RETURN_IF_ERROR(thread_manager->StartInstrumentation(metrics));
             :   RegisterUrlCallbacks(include_jvm_threads, webserver);
             :   return Status::OK();
             : }
If you move these to the end of the file, does that avoid any of the need to 
forward declare RegisterUrlCallbacks() and ThreadOverviewUrlCallback()?


PS3, Line 379: args
long line


-- 
To view, visit http://gerrit.cloudera.org:8080/6013
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id497043ab33dcf107a562f0b1ccd5c46095d397f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to