[kudu-CR] external minicluster: expand EMC dir usage

["Mike Percy (Code Review)" <ge...@cloudera.org>]

Mike Percy has posted comments on this change.

Change subject: external minicluster: expand EMC dir usage
......................................................................


Patch Set 16:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/6845/16/src/kudu/integration-tests/multidir-cluster-itest.cc
File src/kudu/integration-tests/multidir-cluster-itest.cc:

Line 25: #include "kudu/integration-tests/ts_itest-base.h"
nit: alpha order


PS16, Line 40: TabletServerIntegrationTestBase
Would you mind using ExternalMiniClusterITestBase for this test? It's what we've been standardizing on for more recent itests.


Line 83:   NO_FATALS(CreateTable());
You can use TestWorkload.Setup() to easily create a table


Line 103:   InsertPayload(kTableId, 0, 10, 10);
You should be able to get the same effect using TestWorkload.Start() and StopAndJoin() after the ASSERT_EVENTUALLY


Line 104:   SleepFor(kTimeout);
This is potentially flaky. See below for a suggestion.

After making those changes, would you mind running 200 iterations of this test using dist_test.py with --stress_cpu_threads=8 and post the link to the results? LMK if you need help to get that set up and I can show you how to do it.


PS16, Line 107:   int num_dirs_added_to = 0;
              :   for (const string& data_dir : ext_tserver->data_dirs()) {
              :     string data_path = JoinPathSegments(data_dir, "data");
              :     vector<string> files;
              :     inspect_->ListFilesInDir(data_path, &files);
              :     int* num_files_before_insert = FindOrNull(num_files_in_each_dir, data_dir);
              :     ASSERT_NE(nullptr, num_files_before_insert);
              :     if (*num_files_before_insert < files.size()) {
              :       num_dirs_added_to++;
              :     }
              :   }
              :   // Block placement should guarantee that more than one data dir will have
              :   // data written to it.
              :   ASSERT_GT(num_dirs_added_to, 1);
You can wrap this in ASSERT_EVENTUALLY() and avoid the potentially-flaky SleepFor() above


-- 
To view, visit http://gerrit.cloudera.org:8080/6845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2f5def6980ad394c8558ad97ba830f1b0257332
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes