Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17779 )
Change subject: KUDU-2671 update range partitioning with custom hash schema ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/17779/1/src/kudu/common/common.proto File src/kudu/common/common.proto: http://gerrit.cloudera.org:8080/#/c/17779/1/src/kudu/common/common.proto@358 PS1, Line 358: HashBu > If we're changing this to be unsigned, would have to make similar changes e After discussing this offline, the decision was to remove this new data structure and use HashBucketSchemaPB everywhere for consistency. http://gerrit.cloudera.org:8080/#/c/17779/1/src/kudu/common/common.proto@374 PS1, Line 374: // of the table. Otherwise, particular range > Eventually this will be deprecated correct? Yep, my initial idea was to eventually deprecate this field. However, since we need to keep the API backward-compatible, this is to stay at least for a few releases. An alternative approach is to keep this field and use the newly introduced 'custom_hash_schema_ranges' only for the overrides. This alternative approach automatically lets us keep the new servers compatible with old clients and have somewhat shorter requests to create tables when there is a dominant hash schema for the majority of the ranges -- something similar what we have as of now, semantically. Let me know if you think it's better to switch to using only 'custom_hash_schema_ranges' at the new code (at least in the client side as of now). http://gerrit.cloudera.org:8080/#/c/17779/1/src/kudu/common/common.proto@377 PS1, Line 377: : > Should we mark these as deprecated? Done http://gerrit.cloudera.org:8080/#/c/17779/3/src/kudu/common/common.proto File src/kudu/common/common.proto: http://gerrit.cloudera.org:8080/#/c/17779/3/src/kudu/common/common.proto@369 PS3, Line 369: //repeated PerRangeHashBucketSchemasPB DEPRECATED_range_hash_schemas = 3; : //repeated RowOperationsPB DEPRECATED_range_bounds = 4; I guess we could add reserved 3; reserved 4; after removing these fields, but given we haven't released the version with those fields, we can safely re-use the fields is in future versions. Not sure how to proceed here: to be really safe, we should add 'reserved' fields, I guess. -- To view, visit http://gerrit.cloudera.org:8080/17779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37aae56a33170894f30d6cd73a5698d6cbb7a697 Gerrit-Change-Number: 17779 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Sat, 28 Aug 2021 01:47:46 +0000 Gerrit-HasComments: Yes
