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

Change subject: IMPALA-9870: impala-shell 'summary' to show original and 
retried queries
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16502/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16502/1//COMMIT_MSG@12
PS1, Line 12: ALL
> The description below indicates that the number of retry is 1. Will we supp
Yeah, currently we only support one retry. IMPALA-9200 targets on multiple 
retries but it's not planned yet.


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

http://gerrit.cloudera.org:8080/#/c/16502/1/be/src/service/impala-server.h@802
PS1, Line 802: Status GetExecSummary(const TUniqueId& query_id, const 
std::string& user,
             :       TExecSummary* result, TExecSummary* original_result = 
nullptr,
             :       bool* was_retried = nullptr) WARN_UNUSED_RESULT;
> If multiple retries are planned, then this interface may not be sufficient.
Yeah, but I tend to keep this similar to the profile related methods (e.g. 
GetRuntimeProfileOutput() at line 773) since multiple retries are not planned 
yet. We can extend this when we start working on IMPALA-9200.


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

http://gerrit.cloudera.org:8080/#/c/16502/1/be/src/service/impala-server.cc@872
PS1, Line 872: CHECK(original_result != nullptr);
> Seems to me the caller may not be interested to get the summary for the ori
IMO, the caller is expected to pass in a valid pointer of 'original_result' if 
was_retried != nullptr. Because 'was_retried' is used to indicate whether 
'original_result' is filled and can be used. So I add a DCHECK here. Let me 
update the method comment to emphasize this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8605dd0eb2d3a2f64f154afb6c2fd34251c1fec2
Gerrit-Change-Number: 16502
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Qifan Chen <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Sat, 10 Oct 2020 03:53:08 +0000
Gerrit-HasComments: Yes

Reply via email to