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 2019/03/05 00:29:02 UTC
[impala] branch master updated (74dba7a -> 110b362)
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 74dba7a IMPALA-8264: Fix overflow in SystemStateInfo::ComputeCpuRatios
new dc1bc3c IMPALA-8249: End-to-end test framework doesn't read aggregated counters properly
new 43adfac IMPALA-8272: Fix test_catalog_tablesfilesusage failing
new 110b362 IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
The 3 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:
be/src/runtime/coordinator-backend-state.cc | 36 ++++++++++++++++++-----------
tests/common/test_result_verifier.py | 12 ++++++++--
tests/custom_cluster/test_rpc_timeout.py | 14 +++++++----
tests/unittests/test_result_verifier.py | 15 ++++++++++++
tests/webserver/test_web_pages.py | 10 ++++----
5 files changed, 62 insertions(+), 25 deletions(-)
[impala] 02/03: IMPALA-8272: Fix test_catalog_tablesfilesusage
failing
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 43adfac5078780cd939d8ba23d481529dbebf0aa
Author: Yongzhi Chen <yc...@cloudera.com>
AuthorDate: Fri Mar 1 22:54:43 2019 -0500
IMPALA-8272: Fix test_catalog_tablesfilesusage failing
The test can run in any context, do not make any assumption.
Change-Id: I41cfa59882edafcd5e61d2e119cd8e8bff08e544
Reviewed-on: http://gerrit.cloudera.org:8080/12649
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
tests/webserver/test_web_pages.py | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/tests/webserver/test_web_pages.py b/tests/webserver/test_web_pages.py
index c03e143..fd7fe80 100644
--- a/tests/webserver/test_web_pages.py
+++ b/tests/webserver/test_web_pages.py
@@ -302,15 +302,13 @@ class TestWebPage(ImpalaTestSuite):
def __test_catalog_tablesfilesusage(self, db_name, tbl_name, numfiles):
"""Test the list of tables with most number of files in the catalog page.
- Make sure the loaded table is in the list and with correct file number."""
+ Make sure the loaded table has correct file count."""
self.client.execute("refresh %s.%s" % (db_name, tbl_name))
response = self.get_and_check_status(self.CATALOG_URL,
"Tables with Most Number of Files", ports_to_test=self.CATALOG_TEST_PORT)
list_file_str = re.search('<table id="high-file-count-tables"( .*?)</table>',
response[0].text, re.MULTILINE | re.DOTALL)
target_metric = "%s.%s-metric" % (db_name, tbl_name)
- # Check the db table is in the list
- assert target_metric in list_file_str.group(0)
list_files = re.findall('<tr>(.*?)</tr>', list_file_str.group(0),
re.MULTILINE | re.DOTALL)
for trow in list_files:
@@ -324,13 +322,13 @@ class TestWebPage(ImpalaTestSuite):
response_json = json.loads(response[0].text)
high_filecount_tbls = response_json["high_file_count_tables"]
tbl_fname = "%s.%s" % (db_name, tbl_name)
- hasTbl = 0
assert len(high_filecount_tbls) > 0
+ # The expected table might not be in the Top-N list, we may not find it
+ # in the list. Just make sure the file count is right if it is in the
+ # list.
for tblinfo in high_filecount_tbls:
if tblinfo["name"] == tbl_fname:
assert tblinfo["num_files"] == int(numfiles)
- hasTbl = 1
- assert hasTbl == 1
def __run_query_and_get_debug_page(self, query, page_url, query_options=None,
expected_state=None):
[impala] 03/03: IMPALA-8274: Fix iteration of profiles in
ApplyExecStatusReport()
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 110b362a52eda053caa0d177016e22a23a1a9612
Author: Michael Ho <kw...@cloudera.com>
AuthorDate: Sun Mar 3 17:29:21 2019 -0800
IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
The coordinator skips over any stale or duplicated status
reports of fragment instances. In the previous implementation,
the index pointing into the vector of Thrift profiles wasn't
updated when skipping over a status report. This breaks the
assumption that the status reports and thrift profiles vectors
have one-to-one correspondence. Consequently, we may end up
passing the wrong profile to InstanceStats::Update(), leading
to random crashes.
This change fixes the problem above by using iterators to
iterate through the status reports and thrift profiles vectors
and ensures that both iterators are updated on every iteration
of the loop.
Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Reviewed-on: http://gerrit.cloudera.org:8080/12651
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/runtime/coordinator-backend-state.cc | 36 ++++++++++++++++++-----------
tests/custom_cluster/test_rpc_timeout.py | 14 +++++++----
2 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/be/src/runtime/coordinator-backend-state.cc b/be/src/runtime/coordinator-backend-state.cc
index 605a758..8db379f 100644
--- a/be/src/runtime/coordinator-backend-state.cc
+++ b/be/src/runtime/coordinator-backend-state.cc
@@ -310,11 +310,23 @@ bool Coordinator::BackendState::ApplyExecStatusReport(
// If this backend completed previously, don't apply the update.
if (IsDoneLocked(lock)) return false;
- int idx = 0;
- const bool has_profile = thrift_profiles.profile_trees.size() > 0;
- TRuntimeProfileTree empty_profile;
- for (const FragmentInstanceExecStatusPB& instance_exec_status :
- backend_exec_status.instance_exec_status()) {
+ // Use empty profile in case profile serialization/deserialization failed.
+ // 'thrift_profiles' and 'instance_exec_status' vectors have one-to-one correspondance.
+ vector<TRuntimeProfileTree> empty_profiles;
+ vector<TRuntimeProfileTree>::const_iterator profile_iter;
+ if (UNLIKELY(thrift_profiles.profile_trees.size() == 0)) {
+ empty_profiles.resize(backend_exec_status.instance_exec_status().size());
+ profile_iter = empty_profiles.begin();
+ } else {
+ DCHECK_EQ(thrift_profiles.profile_trees.size(),
+ backend_exec_status.instance_exec_status().size());
+ profile_iter = thrift_profiles.profile_trees.begin();
+ }
+
+ for (auto status_iter = backend_exec_status.instance_exec_status().begin();
+ status_iter != backend_exec_status.instance_exec_status().end();
+ ++status_iter, ++profile_iter) {
+ const FragmentInstanceExecStatusPB& instance_exec_status = *status_iter;
int64_t report_seq_no = instance_exec_status.report_seq_no();
int instance_idx = GetInstanceIdx(instance_exec_status.fragment_instance_id());
DCHECK_EQ(instance_stats_map_.count(instance_idx), 1);
@@ -330,10 +342,7 @@ bool Coordinator::BackendState::ApplyExecStatusReport(
}
DCHECK(!instance_stats->done_);
- DCHECK(!has_profile || idx < thrift_profiles.profile_trees.size());
- const TRuntimeProfileTree& profile =
- has_profile ? thrift_profiles.profile_trees[idx++] : empty_profile;
- instance_stats->Update(instance_exec_status, profile, exec_summary,
+ instance_stats->Update(instance_exec_status, *profile_iter, exec_summary,
scan_range_progress);
// Update DML stats
@@ -566,13 +575,14 @@ void Coordinator::BackendState::InstanceStats::Update(
exec_summary_idx =
exec_summary->data_sink_id_to_idx_map[child->metadata().data_sink_id];
}
- TPlanNodeExecSummary& exec_summary = thrift_exec_summary.nodes[exec_summary_idx];
+ TPlanNodeExecSummary& node_exec_summary = thrift_exec_summary.nodes[exec_summary_idx];
+ DCHECK_EQ(node_exec_summary.fragment_idx, exec_params_.fragment().idx);
int per_fragment_instance_idx = exec_params_.per_fragment_instance_idx;
- DCHECK_LT(per_fragment_instance_idx, exec_summary.exec_stats.size())
+ DCHECK_LT(per_fragment_instance_idx, node_exec_summary.exec_stats.size())
<< " name=" << child->name()
<< " instance_id=" << PrintId(exec_params_.instance_id)
<< " fragment_idx=" << exec_params_.fragment().idx;
- TExecStats& instance_stats = exec_summary.exec_stats[per_fragment_instance_idx];
+ TExecStats& instance_stats = node_exec_summary.exec_stats[per_fragment_instance_idx];
RuntimeProfile::Counter* rows_counter = child->GetCounter("RowsReturned");
RuntimeProfile::Counter* mem_counter = child->GetCounter("PeakMemoryUsage");
@@ -580,7 +590,7 @@ void Coordinator::BackendState::InstanceStats::Update(
if (mem_counter != nullptr) instance_stats.__set_memory_used(mem_counter->value());
instance_stats.__set_latency_ns(child->local_time());
// TODO: track interesting per-node metrics
- exec_summary.__isset.exec_stats = true;
+ node_exec_summary.__isset.exec_stats = true;
}
// determine newly-completed scan ranges and update scan_range_progress
diff --git a/tests/custom_cluster/test_rpc_timeout.py b/tests/custom_cluster/test_rpc_timeout.py
index e1a959c..6ff2bf8 100644
--- a/tests/custom_cluster/test_rpc_timeout.py
+++ b/tests/custom_cluster/test_rpc_timeout.py
@@ -127,12 +127,18 @@ class TestRPCTimeout(CustomClusterTestSuite):
self.execute_query_verify_metrics(self.TEST_QUERY, None, 10)
# Inject jitter into the RPC handler of ReportExecStatus() to trigger RPC timeout.
+ # Useful for triggering IMPALA-8274.
@pytest.mark.execute_serially
@CustomClusterTestSuite.with_args("--status_report_interval_ms=100"
- " --backend_client_rpc_timeout_ms=1000")
- def test_reportexecstatus_timeout(self, vector):
- query_options = {'debug_action': 'REPORT_EXEC_STATUS_DELAY:JITTER@1500@0.5'}
- self.execute_query_verify_metrics(self.TEST_QUERY, query_options, 10)
+ " --backend_client_rpc_timeout_ms=100")
+ def test_reportexecstatus_jitter(self, vector):
+ LONG_RUNNING_QUERY = "with v as (select t1.ss_hdemo_sk as xk " +\
+ "from tpcds_parquet.store_sales t1, tpcds_parquet.store_sales t2 " +\
+ "where t1.ss_hdemo_sk = t2.ss_hdemo_sk) " +\
+ "select count(*) from v, tpcds_parquet.household_demographics t3 " +\
+ "where v.xk = t3.hd_demo_sk"
+ query_options = {'debug_action': 'REPORT_EXEC_STATUS_DELAY:JITTER@110@0.7'}
+ self.execute_query_verify_metrics(LONG_RUNNING_QUERY, query_options, 1)
# Use a small service queue memory limit and a single service thread to exercise
# the retry paths in the ReportExecStatus() RPC
[impala] 01/03: IMPALA-8249: End-to-end test framework doesn't read
aggregated counters properly
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 dc1bc3ca03337d6d63d88261226047bb7a55493b
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Tue Feb 26 15:13:06 2019 +0100
IMPALA-8249: End-to-end test framework doesn't read aggregated counters properly
Updated compute_aggregation() function to not read the pretty-printed
value from the runtime profile, but the accurate value which is at
the end of the line in parenthesis, e.g.:
RowsReturned: 2.14M (2142543)
The old regex tried to parse '2.14M' with 'd+', which resulted in '2'
instead of 2142543.
I tested the change manually and added a test case to
'tests/unittests/test_result_verifier.py'.
Change-Id: I2a6fc0d3f7cbaa87aa848cdafffad21fb1514930
Reviewed-on: http://gerrit.cloudera.org:8080/12589
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
tests/common/test_result_verifier.py | 12 ++++++++++--
tests/unittests/test_result_verifier.py | 15 +++++++++++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/tests/common/test_result_verifier.py b/tests/common/test_result_verifier.py
index 8a17028..a9ba7c7 100644
--- a/tests/common/test_result_verifier.py
+++ b/tests/common/test_result_verifier.py
@@ -519,10 +519,18 @@ def compute_aggregation(function, field, runtime_profile):
"""
Evaluate an aggregation function over a field on the runtime_profile. This skips
the averaged fragment and returns the aggregate value. It currently supports only
- integer values and the SUM function.
+ TUnit::UNIT types and the SUM function. It expects the profile to write counters
+ in verbose mode.
"""
start_avg_fragment_re = re.compile('[ ]*Averaged Fragment')
- field_regex = "{0}: (\d+)".format(field)
+ # 'field_regex' matches a TUnit::UNIT field from the runtime profile.
+ # For example, it matches the following line if 'field' is 'RowsReturned':
+ # RowsReturned: 2.14M (2142543)
+ #
+ # These lines are printed by 'be/src/util/pretty-printer.h' with verbose=true.
+ # 'field_regex' also captures the accurate value of the field which is the number
+ # in parenthesis. It means we can retrieve this value with 're.findall()'.
+ field_regex = "{0}: \d+(?:\.\d+[KMB])? \((\d+)\)".format(field)
field_regex_re = re.compile(field_regex)
inside_avg_fragment = False
avg_fragment_indent = None
diff --git a/tests/unittests/test_result_verifier.py b/tests/unittests/test_result_verifier.py
index de8515a..e6bf9ba 100644
--- a/tests/unittests/test_result_verifier.py
+++ b/tests/unittests/test_result_verifier.py
@@ -18,6 +18,7 @@
from tests.common.impala_test_suite import ImpalaTestSuite
from tests.common.test_dimensions import create_uncompressed_text_dimension
from tests.common.test_result_verifier import create_query_result
+from tests.common.test_result_verifier import compute_aggregation
# Unittest class for the test_result_verifier module.
class TestResultVerifier(ImpalaTestSuite):
@@ -55,3 +56,17 @@ class TestResultVerifier(ImpalaTestSuite):
assert False, 'Expected error due to column position not existing'
except IndexError, e:
assert 'list index out of range' in e.message
+
+ def test_compute_aggregation(self, vector):
+ profile = '''
+ FieldA: 5 (5)
+ FieldB: bla bla
+ FieldA: 5.10K (5101)
+ FieldA: 1.11M (1110555)
+ FieldC: 1.23K (1234)
+ FieldA: 2.99B (2990111111)
+ FieldK: 7 (7)
+ '''
+ assert compute_aggregation('SUM', 'FieldA', profile) == 2991226772
+ assert compute_aggregation('SUM', 'FieldK', profile) == 7
+ assert compute_aggregation('SUM', 'FieldX', profile) == 0