impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-2198: Differentiate queries in exceptional states in web UI
Date Fri, 25 Mar 2016 23:26:18 GMT
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-2198: Differentiate queries in exceptional states in web UI
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/2625/1/be/src/service/impala-server-callbacks.cc
File be/src/service/impala-server-callbacks.cc:

Line 303:       num_waiting_queries++;
> single line, prefer pre-increment
Done


http://gerrit.cloudera.org:8080/#/c/2625/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

Line 1599: 
> blank line can be removed
Done


http://gerrit.cloudera.org:8080/#/c/2625/1/be/src/service/impala-server.h
File be/src/service/impala-server.h:

Line 610: eos
> prefer "all_rows_returned"
Done


http://gerrit.cloudera.org:8080/#/c/2625/1/www/queries.tmpl
File www/queries.tmpl:

Line 60:   {{num_waiting_queries}} waiting on client
> this makes it sound like the queries are waiting on *some* client input (wh
I'm open to suggestions of the clearest thing to put here. It seems weird to say something
about them being 'finished', since this is similar to the 'completed queries' section below.
For now I'll change it to 'waiting to be closed'. The difficulty of making this clear was
the motivation behind the tooltip.


Line 61:   <sup><a href='#' data-toggle="tooltip" title="{{waiting-tooltip}}">[?]</a></sup>
> Better just to inline the tooltip text here.
I hadn't done this because I can't figure out any way to wrap an html string in the middle
(i.e. because its over the 90 character line limit) without having the line break show up
in the string when it is displayed. Any idea how to do this?


Line 77:     <th>Action</th>
> It would be good to have a measure of the time a query has been waiting for
The closest thing we already have that I can find would be using the difference between current
time and QueryExecState::last_active_time, though I don't think this is quite the right thing
- for example, if a query has an exception because it timed out, last_active_time isn't updated
to the time when the exception happened, so it would start in the 'waiting' section with a
'waiting time' of about '--idle-query-timeout'

I'm happy to add something that tracks this, if we think its needed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I47d0b642ecb573fefbbf337b8c8f2c479b0d49b2
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message