This is an automated email from the ASF dual-hosted git repository.
csringhofer pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 720b94ad9 IMPALA-13419: Improve test_http_socket_timeout
720b94ad9 is described below
commit 720b94ad992d6bbde0d3c3ec69bd52e6b2d0e63c
Author: Michael Smith <[email protected]>
AuthorDate: Mon Feb 9 15:13:07 2026 -0800
IMPALA-13419: Improve test_http_socket_timeout
Uses a debug action in ExecuteStatement to add a sleep so we can test a
longer http_socket_timeout_s setting, which makes the test more
consistent. Fixes test failures with ASAN builds.
The change in testing means the failure no longer happens during 'SET
ALL' - which was too quick to be reliable - where impala-shell would
then CloseSession after failure. It now happens during executing the
query, which can result in leaving a query behind after the connection
is closed. Moves the test to custom cluster testing because there's no
way to reliably ensure the session is closed after disconnect;
idle_session_timeout should in theory work, but impala-shell has no way
to set it and detecting closed connections takes too long.
Fixes a DCHECK failure exposed by this test where registration fails
because the session has been closed and therefore the query driver was
not finalized:
760958 impala-server.cc:1522] ...] RegisterQuery(): session has been
closed, ignoring query.
760958 query-driver.cc:46] Check failed: finalized_.Load() Finalize()
must have been called
Testing: ran test_http_socket_timeout a bunch of times with ASAN.
Change-Id: Idf31bb1752586ebc296e5e8d62c035bac7371dfb
Reviewed-on: http://gerrit.cloudera.org:8080/23955
Reviewed-by: Csaba Ringhofer <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
---
be/src/runtime/query-driver.cc | 6 ++-
be/src/runtime/query-driver.h | 3 ++
be/src/service/impala-server.cc | 8 +++-
tests/custom_cluster/test_shell_commandline.py | 41 ++++++++++++++++++++
tests/shell/test_shell_commandline.py | 52 --------------------------
5 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/be/src/runtime/query-driver.cc b/be/src/runtime/query-driver.cc
index 05ec95571..b62d46330 100644
--- a/be/src/runtime/query-driver.cc
+++ b/be/src/runtime/query-driver.cc
@@ -548,7 +548,11 @@ Status
QueryDriver::Unregister(ImpalaServer::QueryDriverMap* query_driver_map) {
return Status::OK();
}
-void QueryDriver::IncludeInQueryLog(const bool include) noexcept{
+void QueryDriver::Abandon() {
+ finalized_.Store(true);
+}
+
+void QueryDriver::IncludeInQueryLog(const bool include) noexcept {
include_in_query_log_ = include;
}
diff --git a/be/src/runtime/query-driver.h b/be/src/runtime/query-driver.h
index 9ad0e3e10..f6419da0b 100644
--- a/be/src/runtime/query-driver.h
+++ b/be/src/runtime/query-driver.h
@@ -189,6 +189,9 @@ class QueryDriver {
/// Delete this query from the given QueryDriverMap.
Status Unregister(ImpalaServer::QueryDriverMap* query_driver_map)
WARN_UNUSED_RESULT;
+ /// Abandon this query. This is used in scenarios where the query was never
registered.
+ void Abandon();
+
/// True if Finalize() was called while the query was inflight.
bool finalized() const { return finalized_.Load(); }
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 6ac2646cc..4bd5f0dde 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -1347,8 +1347,12 @@ Status ImpalaServer::Execute(TQueryCtx* query_ctx,
query_handle->query_driver()->IncludeInQueryLog(include_in_query_log);
// Unregister query if it was registered and not yet finalized.
- if (!status.ok() && registered_query &&
!query_handle->query_driver()->finalized()) {
- UnregisterQueryDiscardResult((*query_handle)->query_id(), &status);
+ if (!status.ok() && !query_handle->query_driver()->finalized()) {
+ if (registered_query) {
+ UnregisterQueryDiscardResult((*query_handle)->query_id(), &status);
+ } else {
+ query_handle->query_driver()->Abandon();
+ }
}
return status;
}
diff --git a/tests/custom_cluster/test_shell_commandline.py
b/tests/custom_cluster/test_shell_commandline.py
index 77d5d4d8b..6fd4a02b3 100644
--- a/tests/custom_cluster/test_shell_commandline.py
+++ b/tests/custom_cluster/test_shell_commandline.py
@@ -172,3 +172,44 @@ class TestImpalaShellCommandLine(CustomClusterTestSuite):
for line in log_file:
if line.find("HTTP Connection Tracing Headers") != -1:
pytest.fail("found HTTP connection tracing line line:
{0}".format(line))
+
+ @CustomClusterTestSuite.with_args(cluster_size=1)
+ def test_http_socket_timeout(self, vector):
+ """Test setting different http_socket_timeout_s values. Runs as a custom
cluster test
+ because session is not closed up after the test."""
+ assert vector.get_value('protocol') == 'hs2-http'
+ # Test short http_socket_timeout_s, expect errors. After the connection is
+ # established - which now uses connect_timeout_ms - RPCs in the
minicluster appear to
+ # respond immediately, so non-blocking mode (timeout=0) does not result in
an error.
+ # Instead, use a small timeout that allows OpenSession to succeed, then
fails on
+ # ExecuteStatement due to a 1s sleep via debug action. 100ms timeout is
used to allow
+ # SET ALL to succeed, so we fail predictably. Session and query timeouts
are used to
+ # cleanup the query started by Execute, because after the timeout the
socket is closed
+ # and impala-shell doesn't close the session.
+ debug_action = 'debug_action=EXECUTE_INTERNAL_REGISTERED:SLEEP@1000'
+ args = ['--quiet', '-B', '-Q', debug_action, '--query', 'select 0']
+ result = run_impala_shell_cmd(vector, args +
['--http_socket_timeout_s=0.1'],
+ expect_success=False)
+
+ # Python 2 throws socket.timeout and Python 3 throws TimeoutError.
+ error_template = "[Exception] type=<class '{0}'> in ExecuteStatement.
timed out"
+ err_py2 = error_template.format("socket.timeout")
+ err_py3 = error_template.format("TimeoutError")
+ assert err_py2 in result.stderr or err_py3 in result.stderr, result.stderr
+
+ # Test http_socket_timeout_s=-1, expect errors
+ result = run_impala_shell_cmd(vector, args +
['--http_socket_timeout_s=-1'],
+ expect_success=False)
+ expected_err = ("http_socket_timeout_s must be a nonnegative floating
point number"
+ " expressing seconds, or None")
+ assert expected_err in result.stderr
+
+ # Test http_socket_timeout_s>0, expect success
+ result = run_impala_shell_cmd(vector, args + ['--http_socket_timeout_s=2'])
+ assert result.stderr == ""
+ assert result.stdout == "0\n"
+
+ # Test http_socket_timeout_s=None, expect success
+ result = run_impala_shell_cmd(vector, args +
['--http_socket_timeout_s=None'])
+ assert result.stderr == ""
+ assert result.stdout == "0\n"
diff --git a/tests/shell/test_shell_commandline.py
b/tests/shell/test_shell_commandline.py
index a79b0260d..1f9c454b6 100644
--- a/tests/shell/test_shell_commandline.py
+++ b/tests/shell/test_shell_commandline.py
@@ -63,19 +63,6 @@ RUSSIAN_CHARS = utf8_encode_if_needed(
u"А, Б, В, Г, Д, Е, Ё, Ж, З, И, Й, К, Л, М, Н, О, П, Р,"
u"С, Т, У, Ф, Х, Ц,Ч, Ш, Щ, Ъ, Ы, Ь, Э, Ю, Я")
-"""IMPALA-12216 implemented timestamp to be printed in case of any
error/warning
- during query execution, below is an example :
-
- 2024-07-15 12:49:27 [Exception] type=<class 'socket.error'> in FetchResults.
- 2024-07-15 12:49:27 [Warning] Cancelling Query
- 2024-07-15 12:49:27 [Warning] close session RPC failed: <class
'shell_exceptions.
- QueryCancelledByShellException'>
-
- To avoid test flakiness due to timestamp, we would be ignoring timestamp in
actual
- result before asserting with expected result, (YYYY-MM-DD hh:mm:ss ) is of
length 20
-"""
-TS_LEN = 20
-
def find_query_option(key, string, strip_brackets=True):
"""
@@ -1476,45 +1463,6 @@ class TestImpalaShell(ImpalaTestSuite):
rows_from_file = [line.rstrip() for line in f]
assert rows_from_stdout == rows_from_file
- @pytest.mark.execute_serially
- def test_http_socket_timeout(self, vector):
- """Test setting different http_socket_timeout_s values."""
- if (vector.get_value('strict_hs2_protocol')
- or vector.get_value('protocol') != 'hs2-http'):
- pytest.skip("http socket timeout not supported in strict hs2 mode."
- " Only supported with hs2-http protocol.")
- # Test very short http_socket_timeout_s, expect errors. After the
connection is
- # established - which now uses connect_timeout_ms - RPCs in the
minicluster appear to
- # respond immediately, so non-blocking mode (timeout=0) does not result in
an error.
- # Instead, use a very small timeout that the RPC cannot meet.
- args = ['--quiet', '-B', '--query', 'select 0']
- result = run_impala_shell_cmd(vector, args +
['--http_socket_timeout_s=0.001'],
- expect_success=False)
-
- # Python 2 throws socket.timeout and Python 3 throws TimeoutError.
- error_template = "[Exception] type=<class '{0}'> in ExecuteStatement.
timed out"
- pyver_exceptions = ["socket.timeout", "TimeoutError"]
- actual_err = result.stderr.splitlines()[0]
- assert actual_err[TS_LEN:] in [error_template.format(pever_exception)
- for pever_exception in pyver_exceptions]
-
- # Test http_socket_timeout_s=-1, expect errors
- result = run_impala_shell_cmd(vector, args +
['--http_socket_timeout_s=-1'],
- expect_success=False)
- expected_err = ("http_socket_timeout_s must be a nonnegative floating
point number"
- " expressing seconds, or None")
- assert result.stderr.splitlines()[0] == expected_err
-
- # Test http_socket_timeout_s>0, expect success
- result = run_impala_shell_cmd(vector, args + ['--http_socket_timeout_s=2'])
- assert result.stderr == ""
- assert result.stdout == "0\n"
-
- # Test http_socket_timeout_s=None, expect success
- result = run_impala_shell_cmd(vector, args +
['--http_socket_timeout_s=None'])
- assert result.stderr == ""
- assert result.stdout == "0\n"
-
def test_connect_max_tries(self, vector):
"""Test setting different connect_max_tries values."""
if (vector.get_value('strict_hs2_protocol')
