IMPALA-5853: fix GetResultSetMetadata() error message for invalid query id When given an invalid query id, most RPCs result with "Invalid query id", but GetResultSetMetadata() response with "Unable to find session ID for query handle". That's confusing; make it consistent with other RPCs.
Change-Id: I51eecf353f9cecaf88cecc9392b08ac1b9326b66 Reviewed-on: http://gerrit.cloudera.org:8080/7863 Reviewed-by: Dan Hecht <[email protected]> Tested-by: Impala Public 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/593ec257 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/593ec257 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/593ec257 Branch: refs/heads/master Commit: 593ec25736bffdea11ffceb645a27f27d2eb73f5 Parents: 77e9e26 Author: Dan Hecht <[email protected]> Authored: Mon Aug 28 15:25:03 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Aug 29 02:45:00 2017 +0000 ---------------------------------------------------------------------- be/src/service/impala-hs2-server.cc | 17 ++++------------- be/src/service/impala-server.cc | 13 ------------- be/src/service/impala-server.h | 4 ---- tests/hs2/test_hs2.py | 11 ++++++++++- 4 files changed, 14 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/593ec257/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 6a1b5f4..f32d04e 100644 --- a/be/src/service/impala-hs2-server.cc +++ b/be/src/service/impala-hs2-server.cc @@ -720,19 +720,6 @@ void ImpalaServer::GetResultSetMetadata(TGetResultSetMetadataResp& return_val, request.operationHandle.operationId, &query_id, &secret), SQLSTATE_GENERAL_ERROR); VLOG_QUERY << "GetResultSetMetadata(): query_id=" << PrintId(query_id); - // Look up the session ID (which takes session_state_map_lock_) before taking the query - // exec state lock. - TUniqueId session_id; - if (UNLIKELY(!GetSessionIdForQuery(query_id, &session_id))) { - // No handle was found - HS2_RETURN_ERROR(return_val, - Substitute("Unable to find session ID for query handle: $0", PrintId(query_id)), - SQLSTATE_GENERAL_ERROR); - } - ScopedSessionState session_handle(this); - HS2_RETURN_IF_ERROR(return_val, session_handle.WithSession(session_id), - SQLSTATE_GENERAL_ERROR); - shared_ptr<ClientRequestState> request_state = GetClientRequestState(query_id); if (UNLIKELY(request_state.get() == nullptr)) { VLOG_QUERY << "GetResultSetMetadata(): invalid query handle"; @@ -740,6 +727,10 @@ void ImpalaServer::GetResultSetMetadata(TGetResultSetMetadataResp& return_val, HS2_RETURN_ERROR(return_val, Substitute("Invalid query handle: $0", PrintId(query_id)), SQLSTATE_GENERAL_ERROR); } + ScopedSessionState session_handle(this); + const TUniqueId session_id = request_state->session_id(); + HS2_RETURN_IF_ERROR(return_val, session_handle.WithSession(session_id), + SQLSTATE_GENERAL_ERROR); { lock_guard<mutex> l(*request_state->lock()); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/593ec257/be/src/service/impala-server.cc ---------------------------------------------------------------------- diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc index 41f84b1..fb1d0e6 100644 --- a/be/src/service/impala-server.cc +++ b/be/src/service/impala-server.cc @@ -2053,19 +2053,6 @@ Status CreateImpalaServer(ExecEnv* exec_env, int beeswax_port, int hs2_port, int return Status::OK(); } -bool ImpalaServer::GetSessionIdForQuery(const TUniqueId& query_id, - TUniqueId* session_id) { - DCHECK(session_id != nullptr); - lock_guard<mutex> l(client_request_state_map_lock_); - ClientRequestStateMap::iterator i = client_request_state_map_.find(query_id); - if (i == client_request_state_map_.end()) { - return false; - } else { - *session_id = i->second->session_id(); - return true; - } -} - shared_ptr<ClientRequestState> ImpalaServer::GetClientRequestState( const TUniqueId& query_id) { lock_guard<mutex> l(client_request_state_map_lock_); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/593ec257/be/src/service/impala-server.h ---------------------------------------------------------------------- diff --git a/be/src/service/impala-server.h b/be/src/service/impala-server.h index 344368b..1e83de5 100644 --- a/be/src/service/impala-server.h +++ b/be/src/service/impala-server.h @@ -400,10 +400,6 @@ class ImpalaServer : public ImpalaServiceIf, public ImpalaHiveServer2ServiceIf, std::shared_ptr<ClientRequestState> GetClientRequestState( const TUniqueId& query_id); - /// Writes the session id, if found, for the given query to the output - /// parameter. Returns false if no query with the given ID is found. - bool GetSessionIdForQuery(const TUniqueId& query_id, TUniqueId* session_id); - /// Updates the number of databases / tables metrics from the FE catalog Status UpdateCatalogMetrics() WARN_UNUSED_RESULT; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/593ec257/tests/hs2/test_hs2.py ---------------------------------------------------------------------- diff --git a/tests/hs2/test_hs2.py b/tests/hs2/test_hs2.py index 42c3fb7..f42b29a 100644 --- a/tests/hs2/test_hs2.py +++ b/tests/hs2/test_hs2.py @@ -262,10 +262,19 @@ class TestHS2(HS2TestSuite): TestHS2.check_response(get_operation_status_resp, \ TCLIService.TStatusCode.ERROR_STATUS) - print get_operation_status_resp.status.errorMessage err_msg = "Invalid query handle: efcdab8967452301:3031323334353637" assert err_msg in get_operation_status_resp.status.errorMessage + get_result_set_metadata_req = TCLIService.TGetResultSetMetadataReq() + get_result_set_metadata_req.operationHandle = operation_handle + get_result_set_metadata_resp = \ + self.hs2_client.GetResultSetMetadata(get_result_set_metadata_req) + TestHS2.check_response(get_result_set_metadata_resp, \ + TCLIService.TStatusCode.ERROR_STATUS) + + err_msg = "Invalid query handle: efcdab8967452301:3031323334353637" + assert err_msg in get_result_set_metadata_resp.status.errorMessage + @pytest.mark.execute_serially def test_socket_close_forces_session_close(self): """Test that closing the underlying socket forces the associated session to close.
