impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-1169: Admission control info on the debug webpage
Date Wed, 19 Oct 2016 19:51:51 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-1169: Admission control info on the debug webpage

Patch Set 1:


Nice! This will be useful to a lot of people.
Commit Message:

PS1, Line 7: the
the queries debug page
File be/src/scheduling/

Line 458:   schedule->summary_profile()->AddInfoString(PROFILE_INFO_KEY_ADMISSION_RESULT,
here would be a good place to add an event to indicate Queued
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.
File be/src/service/

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()).
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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I504e3c829a14318721e3a42de6281bcc578f7283
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-HasComments: Yes

View raw message