IMPALA-4488: HS2 GetOperationStatus() should keep session alive GetOperationStatus() is used by *DBC drivers to check if a query is ready for its results to be fetched. However, it did not keep the associated session alive, so queries would time out if they took longer than the timeout to materialize their first rows to be fetched.
* Add withSession() to GetOperationStatus() * Add a test that failed before this patch, and succeeds after. Change-Id: Ibb3f66188209563b4b74b2ca96480f16ace0f190 Reviewed-on: http://gerrit.cloudera.org:8080/5213 Reviewed-by: Alex Behm <[email protected]> Tested-by: Internal Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/e4fc5bd5 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/e4fc5bd5 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/e4fc5bd5 Branch: refs/heads/master Commit: e4fc5bd5c515a73358cdc83e7eed5cd128262380 Parents: 96d98ab Author: Henry Robinson <[email protected]> Authored: Wed Nov 23 16:58:24 2016 -0800 Committer: Internal Jenkins <[email protected]> Committed: Thu Nov 24 05:39:45 2016 +0000 ---------------------------------------------------------------------- be/src/service/impala-hs2-server.cc | 21 +++++++++++++-------- tests/hs2/test_hs2.py | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e4fc5bd5/be/src/service/impala-hs2-server.cc ---------------------------------------------------------------------- diff --git a/be/src/service/impala-hs2-server.cc b/be/src/service/impala-hs2-server.cc index eb96359..ab4d1f3 100644 --- a/be/src/service/impala-hs2-server.cc +++ b/be/src/service/impala-hs2-server.cc @@ -622,10 +622,19 @@ void ImpalaServer::GetOperationStatus(TGetOperationStatusResp& return_val, request.operationHandle.operationId, &query_id, &secret), SQLSTATE_GENERAL_ERROR); VLOG_ROW << "GetOperationStatus(): query_id=" << PrintId(query_id); - lock_guard<mutex> l(query_exec_state_map_lock_); - QueryExecStateMap::iterator entry = query_exec_state_map_.find(query_id); - if (entry != query_exec_state_map_.end()) { - QueryExecState *exec_state = entry->second.get(); + shared_ptr<QueryExecState> exec_state = GetQueryExecState(query_id, false); + if (exec_state.get() == nullptr) { + // No handle was found + HS2_RETURN_ERROR(return_val, "Invalid query handle", SQLSTATE_GENERAL_ERROR); + } + + ScopedSessionState session_handle(this); + const TUniqueId session_id = exec_state->session_id(); + shared_ptr<SessionState> session; + HS2_RETURN_IF_ERROR(return_val, session_handle.WithSession(session_id, &session), + SQLSTATE_GENERAL_ERROR); + + { lock_guard<mutex> l(*exec_state->lock()); TOperationState::type operation_state = QueryStateToTOperationState( exec_state->query_state()); @@ -637,11 +646,7 @@ void ImpalaServer::GetOperationStatus(TGetOperationStatusResp& return_val, } else { DCHECK(exec_state->query_status().ok()); } - return; } - - // No handle was found - HS2_RETURN_ERROR(return_val, "Invalid query handle", SQLSTATE_GENERAL_ERROR); } void ImpalaServer::CancelOperation(TCancelOperationResp& return_val, http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e4fc5bd5/tests/hs2/test_hs2.py ---------------------------------------------------------------------- diff --git a/tests/hs2/test_hs2.py b/tests/hs2/test_hs2.py index 6912b69..0ed9cd3 100644 --- a/tests/hs2/test_hs2.py +++ b/tests/hs2/test_hs2.py @@ -284,6 +284,27 @@ class TestHS2(HS2TestSuite): assert "Sql Statement: GET_SCHEMAS" in profile_page assert "Query Type: DDL" in profile_page + @needs_session(conf_overlay={"idle_session_timeout": "5"}) + def test_get_operation_status_session_timeout(self): + """Regression test for IMPALA-4488: GetOperationStatus() would not keep a session + alive""" + execute_statement_req = TCLIService.TExecuteStatementReq() + execute_statement_req.sessionHandle = self.session_handle + # Choose a long-running query so that it can't finish before the session timeout. + execute_statement_req.statement = """select * from functional.alltypes a + join functional.alltypes b join functional.alltypes c""" + execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) + TestHS2.check_response(execute_statement_resp) + + now = time.time() + # Loop until the session would be timed-out if IMPALA-4488 had not been fixed. + while time.time() - now < 10: + get_operation_status_resp = \ + self.get_operation_status(execute_statement_resp.operationHandle) + # Will fail if session has timed out. + TestHS2.check_response(get_operation_status_resp) + time.sleep(0.1) + def get_log(self, query_stmt): execute_statement_req = TCLIService.TExecuteStatementReq() execute_statement_req.sessionHandle = self.session_handle
