Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16096 )

Change subject: IMPALA-9569: Show progress bar and live_summary of the retried 
query
......................................................................


Patch Set 1:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/16096/1/be/src/service/impala-server.cc@746
PS1, Line 746:         if (query_handle->IsRetriedQuery()) {
> what happens if the above if condition is true? e.g. if 'query_handle->exec
Yes, you are right. The retried query will be re-scheduled and should wait for 
admission control again. Will move this block outside.


http://gerrit.cloudera.org:8080/#/c/16096/1/be/src/service/impala-server.cc@749
PS1, Line 749:           DCHECK(status.ok()) << "Original query handle not 
found!";
> This might actually cause issues. Looking through this method, if GetQueryH
Nice catch! The only error is query_driver not found. I thought it's ok since 
GetQueryHandle uses the same query_id as GetActiveQueryHandle at line 717. 
After digged deeper, I realize there is a race if the query is unregistered 
concurrently between this two lines, then the query driver will be not found in 
the query_driver_map.

I think a better way is getting the original query handle along with the latest 
query handle at line 717 in one call. Will update the codes.


http://gerrit.cloudera.org:8080/#/c/16096/1/be/src/service/impala-server.cc@763
PS1, Line 763:   // Look for the query in completed query log.
> somewhat related to this patch, what happens if you run a query in the impa
Yeah, not just "summary", but "profile" also has the same problem. Created 
IMPALA-9870 for this. I didn't modify this part since it's not used in the 
live_summary code path.

For "profile" command, I think we need to decide whether to return two profiles 
since one profile is already big enough...



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f96919f00e0b64d589efd15b6b5ec82fb725d56
Gerrit-Change-Number: 16096
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Sahil Takiar <[email protected]>
Gerrit-Comment-Date: Fri, 19 Jun 2020 02:52:15 +0000
Gerrit-HasComments: Yes

Reply via email to