You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2024/10/15 23:38:32 UTC
(impala) branch master updated (7a5723c40 -> 0767ae065)
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
from 7a5723c40 IMPALA-13346, IMPALA-13420: Deflake test_read_position_deletes[_orc]
new 19678ae65 IMPALA-13431: Deflake TestLogging.test_excessive_cerr_ignore_pid
new 0767ae065 IMPALA-12908: (Addendum) use RUNTIME_FILTER_WAIT_TIME_MS for tuple cache TPC testing
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
common/thrift/Query.thrift | 2 +-
tests/common/environ.py | 5 ++++
tests/custom_cluster/test_breakpad.py | 30 +++++++++++++++++++-----
tests/query_test/test_tuple_cache_tpc_queries.py | 6 +++++
4 files changed, 36 insertions(+), 7 deletions(-)
(impala) 01/02: IMPALA-13431: Deflake TestLogging.test_excessive_cerr_ignore_pid
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 19678ae65c96b09999cbe728cb0e7e1ae2301fd2
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Sat Oct 12 10:25:39 2024 -0700
IMPALA-13431: Deflake TestLogging.test_excessive_cerr_ignore_pid
On a couple UBSAN runs, test_excessive_cerr_ignore_pid sometimes
fails find the message providing the next path in the last line
of the ERROR log file. The logs aren't preserved, so we don't know
the exact contents of the log file.
This does two things:
1. It changes the test to preserve the log file on failure by
copying from the temporary directory to a directory that
will last past the end of the test. This gives us data to
work with if we see this again.
2. A theory is that an extra line or two of logging could go to
the file after it writes the message with the next path.
This changes the test to check the last 3 lines of the log
file for the message providing the next path.
Testing:
- Ran test with UBSAN
Change-Id: I4745184e983ee5669822059289aab18caf0b72a9
Reviewed-on: http://gerrit.cloudera.org:8080/21926
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
tests/custom_cluster/test_breakpad.py | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/tests/custom_cluster/test_breakpad.py b/tests/custom_cluster/test_breakpad.py
index c8348828e..2ed6438d8 100644
--- a/tests/custom_cluster/test_breakpad.py
+++ b/tests/custom_cluster/test_breakpad.py
@@ -433,12 +433,30 @@ class TestLoggingBase(TestBreakpadBase):
if i < len(paths) - 1:
# check that we print the next_path in last line of this log file
next_path = paths[i + 1]
- with open(curr_path, 'rb') as f:
- f.seek(-2, os.SEEK_END)
- while f.read(1) != b'\n':
- f.seek(-2, os.SEEK_CUR)
- last_line = f.readline().decode()
- assert next_path in last_line
+ with open(curr_path, 'r') as f:
+ lines = f.readlines()
+ # There have been some test failures where the next_path is not in the last
+ # line of the output. One theory is that some output could have gone to the
+ # file after the next_path has been written. This tolerates having the
+ # next path in the last 3 lines. It also preserves the logs if there is a
+ # failure.
+ found_next_path = False
+ NUM_LINES_TO_CHECK = 3
+ for i in range(max(NUM_LINES_TO_CHECK, len(lines))):
+ if next_path in lines[-i]:
+ found_next_path = True
+ break
+ if not found_next_path:
+ # These logs are in a temporary directory. To improve debuggability,
+ # copy the logs to a location that would be preserved.
+ preserved_log_dir = os.getenv("LOG_DIR", "/tmp/")
+ preserved_path = os.path.join(preserved_log_dir,
+ os.path.basename(curr_path))
+ shutil.copyfile(curr_path, preserved_path)
+ msg = "Did not find {0} in the last {1} lines of {2}.".format(
+ next_path, NUM_LINES_TO_CHECK, curr_path)
+ msg += " Preserved the log contents at {0}".format(preserved_path)
+ assert False, msg
except OSError:
# The daemon might delete the log in the middle of assertion.
# In that case, do nothing and move on.
(impala) 02/02: IMPALA-12908: (Addendum) use RUNTIME_FILTER_WAIT_TIME_MS for tuple cache TPC testing
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 0767ae065a9bf3023ad0be3d98ebad0e4d0c32fa
Author: Yida Wu <yi...@cloudera.com>
AuthorDate: Thu Oct 3 14:09:55 2024 -0700
IMPALA-12908: (Addendum) use RUNTIME_FILTER_WAIT_TIME_MS for tuple cache TPC testing
When runtime filters arrive after tuple caching has occurred, they
can't filter the cached results. This can lead to larger tuple caching
result sets than expected, causing correctness check failures in TPC
tests.
While other solutions may exist, extending RUNTIME_FILTER_WAIT_TIME_MS
is a simple fix by ensuring runtime filters are applied before tuple
caching.
Also set the query option enable_tuple_cache_verification to false
by default, as the filter arrival time may affect the correctness
check. To avoid flaky tests, change to use a more conservative
approach and only enable the correctness check when explicitly
specified by the testcase.
Tests:
Verified TPC tests pass correctness checks with increased runtime
filter wait time.
Change-Id: Ie70a87344c436ce8e2073575df5c5bf762ef562d
Reviewed-on: http://gerrit.cloudera.org:8080/21898
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
common/thrift/Query.thrift | 2 +-
tests/common/environ.py | 5 +++++
tests/query_test/test_tuple_cache_tpc_queries.py | 6 ++++++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/common/thrift/Query.thrift b/common/thrift/Query.thrift
index d0b8355a6..e21babc2c 100644
--- a/common/thrift/Query.thrift
+++ b/common/thrift/Query.thrift
@@ -752,7 +752,7 @@ struct TQueryOptions {
183: optional i32 long_polling_time_ms = 0;
// See comment in ImpalaService.thrift
- 184: optional bool enable_tuple_cache_verification = true;
+ 184: optional bool enable_tuple_cache_verification = false;
}
// Impala currently has three types of sessions: Beeswax, HiveServer2 and external
diff --git a/tests/common/environ.py b/tests/common/environ.py
index c1de7e892..165928157 100644
--- a/tests/common/environ.py
+++ b/tests/common/environ.py
@@ -109,6 +109,11 @@ IS_TUPLE_CACHE = (
and os.getenv("TUPLE_CACHE_CAPACITY", "") != ""
)
+# Detect if we are testing with tuple cache correctness check enabled.
+IS_TUPLE_CACHE_CORRECT_CHECK = (
+ os.getenv("TUPLE_CACHE_DEBUG_DUMP_DIR", "") != ""
+)
+
class ImpalaBuildFlavors:
"""
Represents the possible CMAKE_BUILD_TYPE values. These build flavors are needed
diff --git a/tests/query_test/test_tuple_cache_tpc_queries.py b/tests/query_test/test_tuple_cache_tpc_queries.py
index 984121cca..c1f5a9a57 100644
--- a/tests/query_test/test_tuple_cache_tpc_queries.py
+++ b/tests/query_test/test_tuple_cache_tpc_queries.py
@@ -19,6 +19,7 @@
from __future__ import absolute_import, division, print_function
import pytest
+from tests.common.environ import IS_TUPLE_CACHE_CORRECT_CHECK
from tests.common.impala_test_suite import ImpalaTestSuite
from tests.common.skip import SkipIf
from tests.common.test_dimensions import create_single_exec_option_dimension
@@ -29,6 +30,11 @@ MT_DOP_VALUES = [0, 4]
def run_tuple_cache_test(self, vector, query, mtdop):
vector.get_value('exec_option')['enable_tuple_cache'] = True
+ # Use a long runtime filter wait time (1 minute) to ensure filters arrive before
+ # generating the tuple cache for correctness check.
+ if IS_TUPLE_CACHE_CORRECT_CHECK:
+ vector.get_value('exec_option')['runtime_filter_wait_time_ms'] = 60000
+ vector.get_value('exec_option')['enable_tuple_cache_verification'] = True
vector.get_value('exec_option')['mt_dop'] = mtdop
# Run twice to test write and read the tuple cache.
self.run_test_case(query, vector)