impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-1169: Admission control info on the queries debug webpage
Date Fri, 21 Oct 2016 20:42:01 GMT
Thomas Tauber-Marshall has posted comments on this change.

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

Patch Set 3:


As discussed, eliminated the separate list on the webpage in favor of just having the "Queued"
File be/src/scheduling/

PS2, Line 879: Status SimpleScheduler::Schedule
> Can you comment in the header the post-conditions:

PS2, Line 900: // TODO-MT: call AdmitQuery()
> this should also have set_is_admitted(true) until this TODO is resolved.
File be/src/service/

PS2, Line 348: _t wait
> now 'waiting' is a bit ambiguous... can you rename to waiting_for_close now
File be/src/service/query-exec-state.h:

PS2, Line 157:   /// Resource pool associated with this query, or an empty string if the schedule
has not
             :   /// been created and had the pool set yet.
             :   std::string request_pool() const {
             :     return schedule_ == NULL ? "" : schedule_->request_pool();
             :   }
             :   int num_rows_fetched() const { return num_rows_fetched_; }
             :   void set_fetched_rows() { fetched_rows_ = true; }
             :   bool fetched_rows() const { return fetched_rows_; }
             :   b
> I think this is big enough to move it out of the header.

PS2, Line 166:   const TResultSetMetadata* result_metadata() { return &result_metadata_;
             :   const TUniqueId& query_id() const { return query_ctx_.query_id; }
             :   c
> comment here too, when does this return an empty string?
File tests/custom_cluster/

PS2, Line 602: _get_queries_page
> the metrics page exposes json too (and 'metrics' is a bit overloaded), so w

PS2, Line 603: the number of queries currently in the 'admit
> the number of queries currently in the 'admitted' and 'queued' states

PS2, Line 710:       if thread.error is not None:
             :         raise thread.error
> here's where it'd be helpful to differentiate between current counts (i.e. 

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