You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2016/09/07 23:49:04 UTC

incubator-impala git commit: IMPALA-4087: TestFragmentLifecycle.test_failure_in_prepare

Repository: incubator-impala
Updated Branches:
  refs/heads/master 25fe78291 -> 94ef5b19b


IMPALA-4087: TestFragmentLifecycle.test_failure_in_prepare

The test previously got the "initial" value of number of fragments by
reading the metric. This gave an incorrect non-zero result if there were
any leftover queries running on the cluster.

Avoid the problem and simplify the test by explicitly waiting for the
number of fragments to go to zero.

Change-Id: I112e502a25f075928b0f6ef376c7fd9c6376ef4d
Reviewed-on: http://gerrit.cloudera.org:8080/4325
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
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/94ef5b19
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/94ef5b19
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/94ef5b19

Branch: refs/heads/master
Commit: 94ef5b19b9c6f8599ae1787c2d085b54f562689f
Parents: 25fe782
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Sep 7 09:04:15 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Wed Sep 7 23:33:36 2016 +0000

----------------------------------------------------------------------
 tests/query_test/test_lifecycle.py | 13 ++++---------
 tests/verifiers/metric_verifier.py | 10 +---------
 2 files changed, 5 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/94ef5b19/tests/query_test/test_lifecycle.py
----------------------------------------------------------------------
diff --git a/tests/query_test/test_lifecycle.py b/tests/query_test/test_lifecycle.py
index e6b2351..e1269d6 100644
--- a/tests/query_test/test_lifecycle.py
+++ b/tests/query_test/test_lifecycle.py
@@ -24,9 +24,6 @@ from tests.verifiers.metric_verifier import MetricVerifier
 class TestFragmentLifecycle(ImpalaTestSuite):
   """Using the debug action interface, check that failed queries correctly clean up *all*
   fragments"""
-  # TODO: The metric num-fragments-in-flight might not be 0 yet due
-  # to previous tests. Check the value change instead of 0 to reduce
-  # flaky. However, this might not capture some failures.
 
   IN_FLIGHT_FRAGMENTS = "impala-server.num-fragments-in-flight"
   @classmethod
@@ -36,8 +33,7 @@ class TestFragmentLifecycle(ImpalaTestSuite):
   @pytest.mark.execute_serially
   def test_failure_in_prepare(self):
     # Fail the scan node
-    verifiers = [ MetricVerifier(i.service, [self.IN_FLIGHT_FRAGMENTS])
-        for i in ImpalaCluster().impalads ]
+    verifiers = [ MetricVerifier(i.service) for i in ImpalaCluster().impalads ]
     self.client.execute("SET DEBUG_ACTION='-1:0:PREPARE:FAIL'");
     try:
       self.client.execute("SELECT COUNT(*) FROM functional.alltypes")
@@ -46,14 +42,13 @@ class TestFragmentLifecycle(ImpalaTestSuite):
       pass
 
     for v in verifiers:
-      v.wait_for_metric_reset(self.IN_FLIGHT_FRAGMENTS)
+      v.wait_for_metric(self.IN_FLIGHT_FRAGMENTS, 0)
 
   @pytest.mark.execute_serially
   def test_failure_in_prepare_multi_fragment(self):
     # Test that if one fragment fails that the others are cleaned up during the ensuing
     # cancellation.
-    verifiers = [ MetricVerifier(i.service, [self.IN_FLIGHT_FRAGMENTS])
-        for i in ImpalaCluster().impalads ]
+    verifiers = [ MetricVerifier(i.service) for i in ImpalaCluster().impalads ]
     # Fail the scan node
     self.client.execute("SET DEBUG_ACTION='-1:0:PREPARE:FAIL'");
 
@@ -70,4 +65,4 @@ class TestFragmentLifecycle(ImpalaTestSuite):
       # timeout is 60s before they wake up and cancel themselves.
       #
       # TODO: Fix when we have cancellable RPCs.
-      v.wait_for_metric_reset(self.IN_FLIGHT_FRAGMENTS, timeout=65)
+      v.wait_for_metric(self.IN_FLIGHT_FRAGMENTS, 0, timeout=65)

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/94ef5b19/tests/verifiers/metric_verifier.py
----------------------------------------------------------------------
diff --git a/tests/verifiers/metric_verifier.py b/tests/verifiers/metric_verifier.py
index 437410c..d5753fd 100644
--- a/tests/verifiers/metric_verifier.py
+++ b/tests/verifiers/metric_verifier.py
@@ -34,12 +34,9 @@ METRIC_LIST = [
 
 class MetricVerifier(object):
   """Reuseable class that can verify common metrics"""
-  def __init__(self, impalad_service, metrics_list=[]):
+  def __init__(self, impalad_service):
     """Initialize module given an ImpalaService object"""
     self.impalad_service = impalad_service
-    self.initial_metrics = {}
-    for metric in metrics_list:
-      self.initial_metrics[metric] = self.impalad_service.get_metric_value(metric)
 
   def verify_metrics_are_zero(self, timeout=60):
     """Test that all the metric in METRIC_LIST are 0"""
@@ -60,8 +57,3 @@ class MetricVerifier(object):
 
   def wait_for_metric(self, metric_name, expected_value, timeout=60):
     self.impalad_service.wait_for_metric_value(metric_name, expected_value, timeout)
-
-  def wait_for_metric_reset(self, metric_name, timeout=60):
-    if metric_name in self.initial_metrics:
-      self.impalad_service.wait_for_metric_value(metric_name,
-          self.initial_metrics[metric_name], timeout)