Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8434 )
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! http://gerrit.cloudera.org:8080/#/c/8434/1/be/src/service/client-request-state.h File be/src/service/client-request-state.h: http://gerrit.cloudera.org:8080/#/c/8434/1/be/src/service/client-request-state.h@342 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 http://gerrit.cloudera.org:8080/8434 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings 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 <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Thu, 02 Nov 2017 22:32:56 +0000 Gerrit-HasComments: Yes
