impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Zeyliger (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5564: Release lock during planning. (wip)
Date Tue, 31 Oct 2017 22:45:42 GMT
Philip Zeyliger has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8434


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

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

** I'm looking for feedback on the approach here; if folks
think this is the right direction, I'll work on chasing
down why Thrift Exception messages aren't propagated and
adding tests for any Thrift method that takes a query_id,
across Beeswax and HS2 APIs. **

This commit changes the query path to release the client request state
lock during planning. As a result, the plan becomes available to the web
UI, whereas the web UI would previously block on the lock.

This introduces a new boolean, is_planning_, to capture that that we're
in planning. This is done largely to be able to assert that result
metadata is not queried during planning. The common workflow here is:
client, using Thrift, calls ImpalaServer::ExecuteStatement() (in
impala-hs2-server.cc), which calls ImpalaServer::Execute(), which calls
ImpalaServer::ExecuteInternal(), where the plannig happens. Because the
first time the client can cleanly get the query id is the return of
ExecuteStatement(), there shouldn't be a way to know the query id
that's necessary to call GetResultSetMetadata(). If someone reaches
around (e.g., via the web UI), they will get an error message.

This happens to addess one of the points in IMPALA-3882, which
talks about releasing this very lock.

The core tests pass. One exhaustive test had to be modified,
and I saw exhaustive failures that are unrelated.
I added a new test for one Thrift Beeswax API whose behavior
has changed. If folks think this approach is fine, I'll
work through the other APIs.

Change-Id: I1e3fc4c28d7a5ad8546d48bcd22c03fbce502e5b
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-server.cc
M tests/common/impala_connection.py
A tests/custom_cluster/test_profile_during_planning.py
M tests/custom_cluster/test_query_concurrency.py
8 files changed, 147 insertions(+), 26 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/34/8434/1
-- 
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: newchange
Gerrit-Change-Id: I1e3fc4c28d7a5ad8546d48bcd22c03fbce502e5b
Gerrit-Change-Number: 8434
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <philip@cloudera.com>

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