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