Matthew Jacobs has posted comments on this change. Change subject: IMPALA-1169: Admission control info on the debug webpage ......................................................................
Patch Set 1: (7 comments) Nice! This will be useful to a lot of people. http://gerrit.cloudera.org:8080/#/c/4756/1//COMMIT_MSG Commit Message: PS1, Line 7: the the queries debug page http://gerrit.cloudera.org:8080/#/c/4756/1/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: Line 458: schedule->summary_profile()->AddInfoString(PROFILE_INFO_KEY_ADMISSION_RESULT, here would be a good place to add an event to indicate Queued http://gerrit.cloudera.org:8080/#/c/4756/1/be/src/scheduling/query-schedule.h File be/src/scheduling/query-schedule.h: PS1, Line 236: bool is_queued() const { : const std::string* result = : summary_profile_->GetInfoString(PROFILE_INFO_KEY_ADMISSION_RESULT); : return result == NULL ? false : *result == PROFILE_INFO_VAL_QUEUED; : } Inspecting the info string is hacky -- but we can avoid having to provide this interface if the query events include an event right before queueing. Then please move the const strings back to AC as well. http://gerrit.cloudera.org:8080/#/c/4756/1/be/src/service/impala-server.cc File be/src/service/impala-server.cc: PS1, Line 783: Start execution This ends up being misleading when planning takes a while. I think it'll be better to change this to "Registered", and probably to move it closer to line 802 (it shouldn't matter if it's here or right before RegisterQuery()). http://gerrit.cloudera.org:8080/#/c/4756/1/www/queries.tmpl File www/queries.tmpl: PS1, Line 26: {{num_not_scheduled_queries}} queries not scheduled yet we should get input from a few others about naming. 'scheduling' is somewhat overloaded so we may want to change it here and elsewhere. PS1, Line 41: <th>Is Queued?</th> Instead of this printed true/false in a separate column, can you add an event to indicate the query is getting queued? Then it'll show up in Last Event. I think that makes sense there, especially since queuing isn't independent of the events. PS1, Line 151: <h3>Last {{completed_log_size}} Completed Queries</h3> We should show the resource pool in this table as well -- To view, visit http://gerrit.cloudera.org:8080/4756 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I504e3c829a14318721e3a42de6281bcc578f7283 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
