Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10375 )

Change subject: Kudu Backup/Restore Spark Jobs
......................................................................


Patch Set 16:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/10375/16//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10375/16//COMMIT_MSG@22
PS16, Line 22: Additionally in the each tables directory
This still doesn't read correctly; what is "the each tables directory"?


http://gerrit.cloudera.org:8080/#/c/10375/16/java/kudu-backup/src/main/protobuf/backup.proto
File java/kudu-backup/src/main/protobuf/backup.proto:

PS16:
Just remembered that, stylistically, we use underscore_delimited_names for 
proto fields, not camelCase. See src/kudu/common/common.proto for an example.


http://gerrit.cloudera.org:8080/#/c/10375/16/java/kudu-backup/src/main/protobuf/backup.proto@77
PS16, Line 77:     int64 fromMs = 1;
             :     int64 toMs = 2;
What do these mean? Are they quantities of msec from the UNIX epoch? Please doc 
this, and please doc the rest of the messages/fields as I suggested earlier.


http://gerrit.cloudera.org:8080/#/c/10375/16/java/kudu-backup/src/main/protobuf/backup.proto@80
PS16, Line 80:     int32 replicas = 4;
Nit: would prefer num_replicas.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02183a2f833ffa0225eb7b0a35fc7531109e6f7
Gerrit-Change-Number: 10375
Gerrit-PatchSet: 16
Gerrit-Owner: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Wed, 13 Jun 2018 20:33:09 +0000
Gerrit-HasComments: Yes

Reply via email to