Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14578 )
Change subject: [master] KUDU-2963 Fix and enhance TestCreateWhenMajorityOfReplicasFailCreation ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc File src/kudu/integration-tests/create-table-itest.cc: http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc@144 PS3, Line 144: bool in_progress = true; Nice. http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc@151 PS3, Line 151: // At this point, table has been successfully created along with the tablets across the replicas. Nit: "along with the tablets across the replicas" seems like a misnomer: what does it mean for a table to be created "across" replicas? http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc@152 PS3, Line 152: // All the replica tablet servers should be left with only one tablet, eventually, Nit: "replica tablet servers" also reads like a misnomer. http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc@151 PS3, Line 151: // At this point, table has been successfully created along with the tablets across the replicas. : // All the replica tablet servers should be left with only one tablet, eventually, : // since the tablets which failed to get created properly should get deleted. : // It's possible there are some delete tablet RPCs still inflight and not processed yet. As per contributing.adoc: The Kudu team allows line lengths of 100 characters per line, rather than Google's standard of 80. Try to keep under 80 where possible, but you can spill over to 100 or so if necessary. http://gerrit.cloudera.org:8080/#/c/14578/3/src/kudu/integration-tests/create-table-itest.cc@174 PS3, Line 174: std::array<int64_t, kNumReplicas> num_create_attempts_arr{}; Could do the same thing with: vector<int64_t> num_create_attempts(kNumReplicas); std::array isn't wrong per se, it's just pretty unusual. -- To view, visit http://gerrit.cloudera.org:8080/14578 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id9943200dbf330481996e58e27744fe9fb49267f Gerrit-Change-Number: 14578 Gerrit-PatchSet: 3 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 30 Oct 2019 00:37:33 +0000 Gerrit-HasComments: Yes
