impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Zeyliger (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5564: Release lock during planning. (wip)
Date Thu, 02 Nov 2017 22:32:56 GMT
Philip Zeyliger has posted comments on this change. (

Change subject: IMPALA-5564: Release lock during planning. (wip)

Patch Set 1:

(1 comment)

Thanks for the reviews!

I'll report back about cancellations and how this looks in tools like CM that look at profiles.
Cancellations isn't something I considered yet, so thanks for the tip!
File be/src/service/client-request-state.h:
PS1, Line 342: is_planning_
> maybe make that is_planning_finished_ (or similar), and initialize it to fa
I started out with this being planning_finished_ (or some such). I ran into the fact that
some calls (like GetSchemas()) avoid the ExecuteInternal() path entirely: there's no planning
to be done, but yet they also have a ClientRequestState, and they use get_result_metadata()
and so on. In my case, the JDBC tests exposed this very clearly, but many other tests would
have as well.

So, I decided to have state around only the query paths that actually do planning, so as to
avoid saying that "GetSchemas()" has finished planning, since that makes less sense. I could
also think about this as a query state, but changing the query state enums seems much more
delicate than this approach.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e3fc4c28d7a5ad8546d48bcd22c03fbce502e5b
Gerrit-Change-Number: 8434
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <>
Gerrit-Reviewer: Bharath Vissapragada <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Philip Zeyliger <>
Gerrit-Comment-Date: Thu, 02 Nov 2017 22:32:56 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message