Edward Fancher has posted comments on this change. Change subject: KUDU-1932. Run at least one tablet-level test against all block managers. ......................................................................
Patch Set 2: (8 comments) http://gerrit.cloudera.org:8080/#/c/7252/2/src/kudu/integration-tests/ts_recovery-itest.cc File src/kudu/integration-tests/ts_recovery-itest.cc: Line 89: ::testing::Values("--block_manager=log","--block_manager=file")); > You will need to avoid --block_manager=log on macOS. You can do that with s Done. Line 96: // Add the block manager type > Nit: period at end of sentence Added. Line 137: > Nit: spurious extra line Removed. PS2, Line 142: vector<string> extra_tserver_flags = {}; : extra_tserver_flags.emplace_back(GetParam()); > instead of copy / pasting this in each test can we do this in the test cons Ok, I did that. Let me know how that looks. Line 145: > Nit: spurious extra line Removed. Line 161: cluster_->SetFlag(cluster_->tablet_server(0), > If this is a test fix, please split it into its own commit instead of inclu Put back. Line 221: cluster_->tablet_server(0)->mutable_flags()->push_back(parameter_flag); > Are you sure this is required? I think this is only for the dynamic flags w The original test clears the flags (mutable_flags()->clear() above. When the cluster restarted, if the parameter was to use the file block manager, it would switch back to the default, which seems to be the log block manager on linux. The test would then fail, since the current blocks would be unreadable. This is the error I get, if I don't set the flag again: F0628 23:04:27.860929 10891 tablet_server_main.cc:72] Check failed: _s.ok() Bad status: IO error: Failed to load FS layout: Could not open /tmp/run_tha_testnzB5fx/test-tmp/ts_recovery-itest.BlockManagerType_TsRecoveryITest.TestCrashDuringLogReplay_1.1498691061636515-8685/minicluster-data/ts-0/data/data/block_manager_instance: existing data was written using the 'file' block manager; cannot restart with a different block manager 'log' without reformatting http://dist-test.cloudera.org/job?job_id=efan.1498690754.10220 I don't get an error, if I set it again. http://dist-test.cloudera.org/job?job_id=efan.1498688543.24466 PS2, Line 540: { > nit: space before opening brace Added back in. -- To view, visit http://gerrit.cloudera.org:8080/7252 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6bf8c4ef45d907f17366776dace9fecd95120821 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Edward Fancher <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Edward Fancher <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
