Alexey Serbin has posted comments on this change. Change subject: KUDU-871. Support tombstoned voting ......................................................................
Patch Set 6: (7 comments) Glanced through one of the tests. Will take a closer look today. http://gerrit.cloudera.org:8080/#/c/6960/6/src/kudu/integration-tests/tombstoned_voting-stress-test.cc File src/kudu/integration-tests/tombstoned_voting-stress-test.cc: PS6, Line 146: LOG(WARNING) Why warning? Is it something non-regular or non-expected? Would INFO be more appropriate here? Also, is it crucial to have these INFO logs output from the test? PS6, Line 148: LOG(FATAL) nit: would FAIL() be more idiomatic in this case (given it's a test)? PS6, Line 159: WARNING INFO? But I might be missing something here. PS6, Line 197: 2 nit: maybe cluster_->num_tablet_servers() ? PS6, Line 228: voter_thread What happens to the thread if the test fails in one of the ASSERT_OK() below? Consider joining the thread in that case as well (it might be SIGSEGV or something like that otherwise). PS6, Line 232: SleepFor(MonoDelta::FromMilliseconds(500)); Could you add some comments explaining why this delay is necessary? What would happen if this delay were 10 times less? PS6, Line 240: SleepFor(MonoDelta::FromMilliseconds(500)); ditto -- To view, visit http://gerrit.cloudera.org:8080/6960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia19d75b185299443b27f41e468bbae20065e7570 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
