Bankim Bhavsar 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)

> Patch Set 3:
>
> Build Started http://jenkins.kudu.apache.org/job/kudu-gerrit/19350/

 Error Message
 /home/jenkins-slave/workspace/kudu-master/3/src/kudu/integration-tests/create- 
table-itest.cc:171
       Expected: kNumReplicas
       Which is: 3
 To be equal to: num_replicas_found
       Which is: 2

Test failed because table is considered created once majority of replica 
tablets have been successfully created. So it's not necessary for all the 
replica tablets to be created.

I'll update the test.

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.
Ack


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: wh
Done


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.
Done


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:
Done


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:
In this case, the size is fixed and known at compile time so std::array is more 
appropriate.



--
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 18:40:33 +0000
Gerrit-HasComments: Yes

Reply via email to