Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19931 )

Change subject: KUDU-3476: Make replica placement range and table aware
......................................................................


Patch Set 6:

(3 comments)

I just took a first look.

http://gerrit.cloudera.org:8080/#/c/19931/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19931/6//COMMIT_MSG@20
PS6, Line 20:
In addition to a unit test, it would be great to add some sort of a more 
'integrated' test where several tables with many range partitions are being 
created.  Once all the replicas have been placed, check out the resulting 
replica distribution.  To add more fun, thow in some concurrency into the mix, 
i.e. clients that issue CreateTable requests can run concurrently.  You could 
add that into create-table-stress-test.cc or a similar scenario.


http://gerrit.cloudera.org:8080/#/c/19931/6/src/kudu/master/placement_policy-test.cc
File src/kudu/master/placement_policy-test.cc:

http://gerrit.cloudera.org:8080/#/c/19931/6/src/kudu/master/placement_policy-test.cc@68
PS6, Line 68: TabletNumByRangeStartKeyMap
Aren't those supposed to be attributed to a table, and only then to a range 
within a particular table?


http://gerrit.cloudera.org:8080/#/c/19931/6/src/kudu/master/placement_policy-test.cc@1277
PS6, Line 1277: } // namespace master
I guess it would be nice to cover at least the following scenarios as well.

* with initial well-balanced/ideal state of the cluster in terms of replica 
distribution:
  ** add several new range partitions for already existing table
  ** add a new table with several range partitions

* with well-balanced distribution of replicas per tablet server, but very 
imbalanced distribution w.r.t. the table's dimension:
  ** add several new range partitions for an existing table
  ** add a new table with several range partitions



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9caeb8d5547e946bfeb152a99e1ec034c3fa0a0f
Gerrit-Change-Number: 19931
Gerrit-PatchSet: 6
Gerrit-Owner: Mahesh Reddy <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <[email protected]>
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Reviewer: Ádám Bakai <[email protected]>
Gerrit-Comment-Date: Wed, 14 Jun 2023 01:43:03 +0000
Gerrit-HasComments: Yes

Reply via email to