[impala] 03/03: IMPALA-12194: Fix flakiness in DataCacheTest.SetReadOnly

[joemcdonnell]

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 05197c6a4c59437fd2f6ac0402d4f3753f4516eb
Author: Joe McDonnell <joemcdonn...@cloudera.com>
AuthorDate: Thu Jun 8 11:56:37 2023 -0700

    IMPALA-12194: Fix flakiness in DataCacheTest.SetReadOnly
    
    DataCacheTest.SetReadOnly spawns a bunch of threads and
    then sets the cache to read only. The threads do a bunch
    of stores to the cache and then a bunch of reads. The
    test DCHECKs that at least one of the reads will be a cache
    miss, because at least one store can't happen because the
    cache is read only. This is racy, because it is possible for
    the threads to complete all the stores before the cache
    is set to read only.
    
    This modifies the test to spawn half the threads, then set
    the cache read only, then spawn the other half of the threads.
    This guarantees that some threads can't complete their stores,
    so the DCHECK won't fire.
    
    Testing:
     - Ran DataCacheTest.SetReadOnly* in a loop for 1000 iterations
    
    Change-Id: Id086c8be27200965c60f716b7303a0627b769281
    Reviewed-on: http://gerrit.cloudera.org:8080/20026
    Reviewed-by: Joe McDonnell <joemcdonn...@cloudera.com>
    Tested-by: Joe McDonnell <joemcdonn...@cloudera.com>
---
 be/src/runtime/io/data-cache-test.cc | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/be/src/runtime/io/data-cache-test.cc 
b/be/src/runtime/io/data-cache-test.cc
index 61e14f89a..8735bf518 100644
--- a/be/src/runtime/io/data-cache-test.cc
+++ b/be/src/runtime/io/data-cache-test.cc
@@ -135,6 +135,7 @@ class DataCacheBaseTest : public testing::Test {
     vector<unique_ptr<Thread>> threads;
     int num_misses[NUM_THREADS];
     int64_t write_times_us[NUM_THREADS];
+    int64_t num_writes = 0;
     for (int i = 0; i < NUM_THREADS; ++i) {
       unique_ptr<Thread> thread;
       num_misses[i] = 0;
@@ -144,13 +145,12 @@ class DataCacheBaseTest : public testing::Test {
              use_per_thread_filename ? thread_name : "", cache, 
max_start_offset,
              &barrier, &num_misses[i], &write_times_us[i]), &thread));
       threads.emplace_back(std::move(thread));
-    }
-
-    int64_t num_writes = 0;
-    if (test_readonly) {
-      // Set read-only before the writes are complete, and record the total 
write count of
-      // the cache after read-only set.
-      num_writes = cache->SetDataCacheReadOnly();
+      // If testing read only, set it read only after starting half of the 
threads.
+      if (i == (NUM_THREADS / 2) && test_readonly) {
+        // Set read-only before the writes are complete, and record the total 
write count
+        // of the cache after read-only set.
+        num_writes = cache->SetDataCacheReadOnly();
+      }
     }
 
     int cache_misses = 0;