[Impala-ASF-CR] IMPALA-5564: Release lock during planning. (wip)

["Philip Zeyliger (Code Review)" <ge...@cloudera.org>]

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:

Took me a long time to get back to this, but I'm looking at it again.

The queries show up fine in CM. Things like "Query Type" still say N/A, but that makes sense. Important things like user and so on show up fine.

Cancellation, however, is tricky. If we do nothing, cancellation will fail with "Query not yet running." For Dan, this may be a trip down memory lane (IMPALA-1178, "fix Impalad crashes during query cancellation", commit f18646286f49a06103f863bbae6f4dd61d10f1eb). In short, we plan the query first, and then mark it "inflight" only later. Because there are races if you cancel a query while it's still setting itself up, we refuse to do the cancellation.

I can change Cancel() to either actually Cancel() (if the query is in flight) or to say "should be cancelled later" and do the actual cancellation later, perhaps at ImpalaServer::SetQueryInflight. This approach also opens the door to cancelling queries during planning. For example, it's possible that while the analyzer is waiting for the catalog/statestore for schema information, it could see if it should abort.

The downside is that this further complicates an already complicated state machine. client-request-state has is_cancelled_, query_state_, eos_ already, and that's not including the session.

For the user, this would mean that a cancelled query still exists for a while, until we get around to noticing that it needs to be taken down. I think that's already true.

Dan, do you think the "mark_for_cancellation" approach is a good direction? 

Thanks!


-- 
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 <ph...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Feb 2018 23:43:49 +0000
Gerrit-HasComments: No