[impala] 04/04: IMPALA-8496: Fix flakiness of test_data_cache.py

[kw...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

kwho pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 25db9ea8f3f4de3086610ccb6040a101691e6340
Author: Michael Ho <kw...@cloudera.com>
AuthorDate: Sat May 4 01:09:55 2019 -0700

    IMPALA-8496: Fix flakiness of test_data_cache.py
    
    test_data_cache.py was added as part of IMPALA-8341 to verify
    that the DataCache hit / miss counts and the DataCache metrics
    are working as expected. The test seems to fail intermittently
    due to unexpected cache misses.
    
    Part of the test creates a temp table from tpch_parquet.lineitem
    and uses it to join against tpch_parquet.lineitem itself on the
    l_orderkey column. The test expects a complete cache hit for
    tpch_parquet.lineitem when joining against the temp table as it
    should be cached entirely as part of CTAS statement. However, this
    doesn't work as expected all the time. In particular, the data cache
    internally divides up the key space into multiple shards and a key
    is hashed to determine the shard it belongs to. By default, the
    number of shards is the same as number of CPU cores (e.g. 16 for AWS
    m5-4xlarge instance). Since the cache size is set to 500MB, each shard
    will have a capacity of 31MB only. In some cases, it's possible that
    some rows of l_orderkey are evicted if the shard they belong to grow
    beyond 31MB. The problem is not deterministic as part of the cache key
    is the modification time of the file, which changes from run-to-run as
    it's essentially determined by the data loading time of the job. This
    leads to flakiness of the test.
    
    To fix this problem, this patch forces the data cache to use a single
    shard only for determinisim. In addition, the test is also skipped for
    non-HDFS and HDFS erasure encoding builds as it's dependent on the scan
    range assignment. To exercise the cache more extensively, the plan is
    to enable it by default for S3 builds instead of relying on BE and E2E
    tests only.
    
    Testing done:
    - Ran test_data_cache.py 10+ times, each with different mtime
      for tpch_parquet.lineitem; Used to fail 2 out of 3 runs.
    
    Change-Id: I98d5b8fa1d3fb25682a64bffaf56d751a140e4c9
    Reviewed-on: http://gerrit.cloudera.org:8080/13242
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../queries/QueryTest/data-cache.test              |  4 ++-
 tests/custom_cluster/test_data_cache.py            | 38 +++++++++++++++++++---
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/testdata/workloads/functional-query/queries/QueryTest/data-cache.test b/testdata/workloads/functional-query/queries/QueryTest/data-cache.test
index 4cd1062..3f0564e 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/data-cache.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/data-cache.test
@@ -9,6 +9,8 @@ row_regex: .*DataCacheMissCount: 64 \(64\).*
 ====
 ---- QUERY
 select count(*) from tpch_parquet.lineitem t1, test_parquet t2 where t1.l_orderkey = t2.l_orderkey;
+---- RESULTS
+30012985
 ---- RUNTIME_PROFILE
 # Exepct cache hits for t1 and cache misses for t2.
 row_regex: .*DataCacheHitCount: 6 \(6\).*
@@ -36,7 +38,7 @@ row_regex: .*DataCachePartialHitCount: 0 \(0\).*
 insert overwrite test_parquet select * from tpch_parquet.lineitem where l_shipmode = 'AIR';
 ====
 ---- QUERY
-# Verifies that stale data from the cache is not used due to change in mtime.
+# Verifies that stale data from the cache is not used.
 select count(distinct l_orderkey) from test_parquet;
 ---- RESULTS
 652393
diff --git a/tests/custom_cluster/test_data_cache.py b/tests/custom_cluster/test_data_cache.py
index f8db6b0..88464bb 100644
--- a/tests/custom_cluster/test_data_cache.py
+++ b/tests/custom_cluster/test_data_cache.py
@@ -18,11 +18,16 @@
 import pytest
 
 from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
+from tests.common.skip import SkipIf, SkipIfEC
 
 
+@SkipIf.not_hdfs
+@SkipIfEC.scheduling
 class TestDataCache(CustomClusterTestSuite):
   """ This test enables the data cache and verfies that cache hit and miss counts
-  in the runtime profile and metrics are as expected.
+  in the runtime profile and metrics are as expected. Run on non-EC HDFS only as
+  this test checks the number of data cache hit counts, which implicitly relies
+  on the scheduler's behavior and number of HDFS blocks.
   """
   @classmethod
   def get_workload(self):
@@ -30,14 +35,37 @@ class TestDataCache(CustomClusterTestSuite):
 
   @pytest.mark.execute_serially
   @CustomClusterTestSuite.with_args(
-      impalad_args="--always_use_data_cache=true --data_cache_write_concurrency=64",
+      impalad_args="--always_use_data_cache=true --data_cache_write_concurrency=64"
+      " --cache_force_single_shard=true",
       start_args="--data_cache_dir=/tmp --data_cache_size=500MB", cluster_size=1)
-  def test_data_cache(self, vector, unique_database):
+  def test_data_cache_deterministic(self, vector, unique_database):
       """ This test creates a temporary table from another table, overwrites it with
-      some other data and verifies that no stale data is read from the cache. Runs
-      with a single node to make it easier to verify the runtime profile.  """
+      some other data and verifies that no stale data is read from the cache. Runs with
+      a single node to make it easier to verify the runtime profile. Also enables higher
+      write concurrency and uses a single shard to avoid non-determinism.
+      """
       self.run_test_case('QueryTest/data-cache', vector, unique_database)
       assert self.get_metric('impala-server.io-mgr.remote-data-cache-dropped-bytes') >= 0
       assert self.get_metric('impala-server.io-mgr.remote-data-cache-hit-bytes') > 0
       assert self.get_metric('impala-server.io-mgr.remote-data-cache-miss-bytes') > 0
       assert self.get_metric('impala-server.io-mgr.remote-data-cache-total-bytes') > 0
+
+  @pytest.mark.execute_serially
+  @CustomClusterTestSuite.with_args(
+      impalad_args="--always_use_data_cache=true",
+      start_args="--data_cache_dir=/tmp --data_cache_size=500MB", cluster_size=1)
+  def test_data_cache(self, vector):
+      """ This test scans the same table twice and verifies the cache hit count metrics
+      are correct. The exact number of bytes hit is non-deterministic between runs due
+      to different mtime of files and multiple shards in the cache.
+      """
+      QUERY = "select * from tpch_parquet.lineitem"
+      # Do a first run to warm up the cache. Expect no hits.
+      self.execute_query(QUERY)
+      assert self.get_metric('impala-server.io-mgr.remote-data-cache-hit-bytes') == 0
+      assert self.get_metric('impala-server.io-mgr.remote-data-cache-miss-bytes') > 0
+      assert self.get_metric('impala-server.io-mgr.remote-data-cache-total-bytes') > 0
+
+      # Do a second run. Expect some hits.
+      self.execute_query(QUERY)
+      assert self.get_metric('impala-server.io-mgr.remote-data-cache-hit-bytes') > 0