Yao Xu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13632 )

Change subject: KUDU-2823 Place tablet replicas based on deminsion
......................................................................


Patch Set 9:

(10 comments)

I refactored this patch to make it more generic. We can now specify dimensions 
for the newly created tablet when creating table or adding range partition.

http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/integration-tests/create-table-itest.cc
File src/kudu/integration-tests/create-table-itest.cc:

http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/integration-tests/create-table-itest.cc@223
PS7, Line 223:  set
> 'hot' or 'cold' depends on the use case.  As I understand, this test verifi
Done


http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/catalog_manager.cc@227
PS7, Line 227: place
> places
Done


http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/catalog_manager.cc@228
PS7, Line 228:  on the number of tablets at a "
             :             "tablet server
> the number of tablets at a tablet server
Done


http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/placement_policy.h
File src/kudu/master/placement_policy.h:

http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/placement_policy.h@78
PS7, Line 78:  on the number of tablets at a tablet
> the number of tablets at a tablet server
Done


http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/placement_policy.h@77
PS7, Line 77: place tablet replicas based on the number of tablets in a
            :   //                dimen
> That sounds a bit vague to me.  What about:
Done


http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/placement_policy.h@91
PS7, Line 91:  place tablet replicas based on the number of tablets in a
            :   //               dime
> ditto
Done


http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/placement_policy.h@92
PS7, Line 92: d on the number of tablets at a table
> the number of tables at a tablet server
Done


http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/ts_descriptor.h
File src/kudu/master/ts_descriptor.h:

http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/master/ts_descriptor.h@61
PS7, Line 61: ens
> Why not unordered_map?  Is there any significance in the order of the keys
Done


http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/tserver/ts_tablet_manager.cc@1250
PS7, Line 1250: d_lock<RWMutex> l
> style nit: const auto& entry
Done


http://gerrit.cloudera.org:8080/#/c/13632/7/src/kudu/tserver/ts_tablet_manager.cc@1260
PS7, Line 1260:   }
              :   *num_live_tablets_by_dimension = std::move(result);
              : }
              :
> result[range]++ ought to work too, right?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I48a225e221eb42ef2f5489687e80a151d8dc1a42
Gerrit-Change-Number: 13632
Gerrit-PatchSet: 9
Gerrit-Owner: Yao Xu <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Yao Xu <[email protected]>
Gerrit-Comment-Date: Wed, 26 Jun 2019 16:05:21 +0000
Gerrit-HasComments: Yes

Reply via email to