You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2017/06/16 21:44:01 UTC

[3/3] kudu git commit: KUDU-2003. Fix fd-cache related tests on high-core systems

KUDU-2003. Fix fd-cache related tests on high-core systems

These two tests would fail on systems with a number of cores that was
not a divisor of 192. This patch switches the tests to use a non-sharded
LRU cache so that the behavior is not dependent on the core count.

I verified that these tests now pass on a machine with 88 logical cores.

Change-Id: Ibd901730f16c70bb2e4a60464ab08a9a6f14c32f
Reviewed-on: http://gerrit.cloudera.org:8080/7160
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/c0798a94
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/c0798a94
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/c0798a94

Branch: refs/heads/master
Commit: c0798a9426581629d285ebf164f112d9a6a2f7ea
Parents: 5808e6d
Author: Todd Lipcon <to...@cloudera.com>
Authored: Mon Jun 12 15:57:45 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Jun 16 21:27:53 2017 +0000

----------------------------------------------------------------------
 src/kudu/fs/block_manager-stress-test.cc |  5 ++++
 src/kudu/util/file_cache-stress-test.cc  | 35 +++++++++++++++------------
 2 files changed, 24 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c0798a94/src/kudu/fs/block_manager-stress-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/block_manager-stress-test.cc b/src/kudu/fs/block_manager-stress-test.cc
index 55de36c..ac23296 100644
--- a/src/kudu/fs/block_manager-stress-test.cc
+++ b/src/kudu/fs/block_manager-stress-test.cc
@@ -37,6 +37,7 @@
 #include "kudu/util/test_util.h"
 #include "kudu/util/thread.h"
 
+DECLARE_bool(cache_force_single_shard);
 DECLARE_double(log_container_excess_space_before_cleanup_fraction);
 DECLARE_double(log_container_live_metadata_before_compact_ratio);
 DECLARE_int64(block_manager_max_open_files);
@@ -115,6 +116,10 @@ class BlockManagerStressTest : public KuduTest {
     // Compact block manager metadata aggressively.
     FLAGS_log_container_live_metadata_before_compact_ratio = 0.99;
 
+    // Use a single cache shard. Otherwise, the cache can be a little bit "sloppy"
+    // depending on the number of CPUs on the system.
+    FLAGS_cache_force_single_shard = true;
+
     if (FLAGS_block_manager_paths.empty()) {
       data_dirs_.push_back(test_dir_);
     } else {

http://git-wip-us.apache.org/repos/asf/kudu/blob/c0798a94/src/kudu/util/file_cache-stress-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/file_cache-stress-test.cc b/src/kudu/util/file_cache-stress-test.cc
index 966f202..807d85e 100644
--- a/src/kudu/util/file_cache-stress-test.cc
+++ b/src/kudu/util/file_cache-stress-test.cc
@@ -54,7 +54,7 @@
     const Status& _s = (to_call);                                         \
     if (!_s.ok()) {                                                       \
       LOG(INFO) << "Dumping cache contents";                              \
-      vector<string> lines = strings::Split(cache_.ToDebugString(), "\n", \
+      vector<string> lines = strings::Split(cache_->ToDebugString(), "\n", \
                                             strings::SkipEmpty());        \
       for (const auto& l : lines) {                                       \
         LOG(INFO) << l;                                                   \
@@ -63,15 +63,12 @@
     CHECK(_s.ok()) << "Bad status: " << _s.ToString();                    \
   } while (0);
 
-// This default value is friendly to many n-CPU configurations.
-DEFINE_int32(test_max_open_files, 192, "Maximum number of open files enforced "
-             "by the cache. Should be a multiple of the number of CPUs on the "
-             "system.");
-
 DEFINE_int32(test_num_producer_threads, 1, "Number of producer threads");
 DEFINE_int32(test_num_consumer_threads, 4, "Number of consumer threads");
 DEFINE_int32(test_duration_secs, 2, "Number of seconds to run the test");
 
+DECLARE_bool(cache_force_single_shard);
+
 using std::deque;
 using std::shared_ptr;
 using std::thread;
@@ -82,22 +79,28 @@ using strings::Substitute;
 
 namespace kudu {
 
+// FD limit to enforce during the test.
+static const int kTestMaxOpenFiles = 100;
+
 template <class FileType>
 class FileCacheStressTest : public KuduTest {
  public:
   typedef unordered_map<string, unordered_map<string, int>> MetricMap;
 
   FileCacheStressTest()
-      : cache_("test",
-               env_,
-               FLAGS_test_max_open_files,
-               scoped_refptr<MetricEntity>()),
-        rand_(SeedRandom()),
+      : rand_(SeedRandom()),
         running_(1) {
+    // Use a single shard. Otherwise, the cache can be a little bit "sloppy"
+    // depending on the number of CPUs on the system.
+    FLAGS_cache_force_single_shard = true;
+    cache_.reset(new FileCache<FileType>("test",
+                                         env_,
+                                         kTestMaxOpenFiles,
+                                         scoped_refptr<MetricEntity>()));
   }
 
   void SetUp() override {
-    ASSERT_OK(cache_.Init());
+    ASSERT_OK(cache_->Init());
   }
 
   void ProducerThread() {
@@ -153,7 +156,7 @@ class FileCacheStressTest : public KuduTest {
           continue;
         }
         shared_ptr<FileType> new_file;
-        TEST_CHECK_OK(cache_.OpenExistingFile(to_open, &new_file));
+        TEST_CHECK_OK(cache_->OpenExistingFile(to_open, &new_file));
         FinishedOpen(to_open);
         metrics[BaseName(to_open)]["open"]++;
         files.emplace_back(new_file);
@@ -177,7 +180,7 @@ class FileCacheStressTest : public KuduTest {
         if (!GetRandomFile(DELETE, &rand, &to_delete)) {
           continue;
         }
-        TEST_CHECK_OK(cache_.DeleteFile(to_delete));
+        TEST_CHECK_OK(cache_->DeleteFile(to_delete));
         metrics[BaseName(to_delete)]["delete"]++;
       }
     } while (!running_.WaitFor(MonoDelta::FromMilliseconds(1)));
@@ -281,7 +284,7 @@ class FileCacheStressTest : public KuduTest {
     }
   }
 
-  FileCache<FileType> cache_;
+  unique_ptr<FileCache<FileType>> cache_;
 
   // Used to seed per-thread PRNGs.
   ThreadSafeRandom rand_;
@@ -343,7 +346,7 @@ TYPED_TEST(FileCacheStressTest, TestStress) {
   // Start the threads.
   PeriodicOpenFdChecker checker(
       this->env_,
-      FLAGS_test_max_open_files +       // cache capacity
+      kTestMaxOpenFiles +               // cache capacity
       FLAGS_test_num_producer_threads + // files being written
       FLAGS_test_num_consumer_threads); // files being opened
   checker.Start();