Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18753 )
Change subject: KUDU-2671: Backup/restore for custom hash schema ...................................................................... Patch Set 4: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/18753/3/java/kudu-backup-common/src/main/protobuf/backup.proto File java/kudu-backup-common/src/main/protobuf/backup.proto: http://gerrit.cloudera.org:8080/#/c/18753/3/java/kudu-backup-common/src/main/protobuf/backup.proto@94 PS3, Line 94: RangePartitionWithHashPartitionMetadataPB nit: PartitionXPartition looks a bit long, how about renaming this into RangeWithCustomHashSchemaMetadataPB or RangeAndHashPartitionMetadataPB ? http://gerrit.cloudera.org:8080/#/c/18753/3/java/kudu-backup-common/src/main/protobuf/backup.proto@96 PS3, Line 96: hash_partitions nit: we used to call structures like this 'hash_schema' in other protobuf files in src/kudu/common/common.proto Is anything specific why we call this 'hash_partitions' here? http://gerrit.cloudera.org:8080/#/c/18753/4/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala File java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala: http://gerrit.cloudera.org:8080/#/c/18753/4/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala@405 PS4, Line 405: rangeWithHashSchema nit: is this actually a collection/list? Then maybe name it 'rangesWithHashSchemas' -- To view, visit http://gerrit.cloudera.org:8080/18753 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7b8e0481e4e9bc0ac0e5fcbf085976325dd55b0d Gerrit-Change-Number: 18753 Gerrit-PatchSet: 4 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Fri, 22 Jul 2022 20:07:07 +0000 Gerrit-HasComments: Yes
