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 queries debug webpage
Date Fri, 21 Oct 2016 22:01:21 GMT
Matthew Jacobs has posted comments on this change.

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

Patch Set 3:


even w/o the new section, how does compute stats behave?
File be/src/scheduling/

Line 56:     request_pool_(""),
was this intentional? i don't see why this is necessary
File be/src/scheduling/

Line 900:     // TODO-MT: call AdmitQuery()
call AdmitQuery() rather than always setting is_admitted
File be/src/scheduling/simple-scheduler.h:

Line 92:   /// If Schedule() returns OK, schedule::is_admitted will be true.
Sorry, on second thought this doesn't add much. There's a comment in the base class where
this information might be better suited, but I can't think of a way to phrase this right now
that I think makes it worthwhile.
File be/src/service/query-exec-state.h:

PS3, Line 158:  and had the pool set yet.
, not had the pool set yet, or this is StmtType doesn't go through admission control.
File tests/common/

PS3, Line 83:   def get_webpage_json(self, page_name):
            :     """Returns the json for the given Impala debug webpage, eg. '/queries'"""
            :     return json.loads(self.read_debug_webpage(page_name + "?json"))
can you move this up to be under read_debug_webpage and call it get_debug_webpage_json?
File tests/custom_cluster/

PS3, Line 603: the 'admitted' and 'queued' states

PS3, Line 658:  metrics from the webpage
counts from the queries page

Line 663: 
would be nice to add a fn that gets the queries page json and checks that all queries (both
in flight and completed) either:

a) have query type QUERY or DML and the pool is self.pool_name
b) has an empty pool name (I think it'd be DDL or SET)

I think the results should be stable to check here (they're not making state transitions)

Line 708: 
the function to check the pool names could be called here too

To view, visit
To unsubscribe, visit

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

View raw message