Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21074 )

Change subject: IMPALA-12602: Unregister queries on idle timeout
......................................................................


Patch Set 7: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21074/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21074/3//COMMIT_MSG@13
PS3, Line 13: Adds a new structure - idle_query_statuses_ - to retain Status 
messages
            : for queries closed this way so that we can continue to return a 
clear
            : error message if the client returns and requests query status or
            : attempts to fetch results. This structure must be global because 
HS2
            : server can only identify a session ID from a query handle, and 
the quer
> Done
Some brain dump about this topic, I don't mean to change anything in this 
review:

While Impala can't look up the session based on the query id in HS2 at the 
moment, I am not convinced that it has to be this way.

The session and query id are completely independent 16 byte uuids at the moment 
(with 4 bytes zeroed in case of the the query id to leave space for the 
fragment instance id).
At the same time the secret (also 16 byte uuid) is the same in the query and 
the session, so actually a secret->session map could allow retrieving the 
session without looking up the query state first.

Another way would to use less bytes for the ids so that both the session + 
query + fragment instance id would fit to 16 bytes.


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

http://gerrit.cloudera.org:8080/#/c/21074/7/be/src/service/impala-server.h@181
PS7, Line 181: /// 2. SessionState::lock
Can you add idle_query_statuses_lock_ to this list?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacfc285ed3587892c7ec6f7df3b5f71c9e41baf0
Gerrit-Change-Number: 21074
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <michael.sm...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 11:32:21 +0000
Gerrit-HasComments: Yes

Reply via email to