[kudu-CR] KUDU-3090 Support backing up ownership info
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16126 ) Change subject: KUDU-3090 Support backing up ownership info .. KUDU-3090 Support backing up ownership info Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Reviewed-on: http://gerrit.cloudera.org:8080/16126 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke --- M java/kudu-backup-common/src/main/protobuf/backup.proto M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java 4 files changed, 10 insertions(+), 2 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 9 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3090 Support backing up ownership info
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16126 ) Change subject: KUDU-3090 Support backing up ownership info .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 10 Jul 2020 13:48:48 + Gerrit-HasComments: No
[kudu-CR] KUDU-3090 Support backing up ownership info
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/16126 )
Change subject: KUDU-3090 Support backing up ownership info
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16126/7/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File
java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/16126/7/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@747
PS7, Line 747: def validateTablesMatch(tableA: String, tableB: String): Unit
= {
> I'm not sure what you mean
Do you mean it was changed between a full backup and a subsequent incremental
backup?
Metadata changes like that aren't super relevant to test because we always use
the metadata from the last incremental backup when restoring.
--
To view, visit http://gerrit.cloudera.org:8080/16126
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143
Gerrit-Change-Number: 16126
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 08 Jul 2020 14:06:50 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3090 Support backing up ownership info
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16126 ) Change subject: KUDU-3090 Support backing up ownership info .. Patch Set 8: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 08 Jul 2020 14:07:08 + Gerrit-HasComments: No
[kudu-CR] KUDU-3090 Support backing up ownership info
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/16126 )
Change subject: KUDU-3090 Support backing up ownership info
..
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16126/7/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File
java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/16126/7/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@747
PS7, Line 747: def validateTablesMatch(tableA: String, tableB: String): Unit
= {
> Does it also make sense to test restoring a table that has had its owner ch
I'm not sure what you mean
http://gerrit.cloudera.org:8080/#/c/16126/7/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@750
PS7, Line 750: assertEquals(tA.getOwner, tB.getOwner)
> nit: maybe also check that the owner isn't empty?
Done
--
To view, visit http://gerrit.cloudera.org:8080/16126
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143
Gerrit-Change-Number: 16126
Gerrit-PatchSet: 8
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 08 Jul 2020 12:17:00 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3090 Support backing up ownership info
Hello Kudu Jenkins, Andrew Wong, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16126 to look at the new patch set (#8). Change subject: KUDU-3090 Support backing up ownership info .. KUDU-3090 Support backing up ownership info Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 --- M java/kudu-backup-common/src/main/protobuf/backup.proto M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java 4 files changed, 10 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/16126/8 -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3090 Support backing up ownership info
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/16126 )
Change subject: KUDU-3090 Support backing up ownership info
..
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16126/7/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala
File
java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala:
http://gerrit.cloudera.org:8080/#/c/16126/7/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@747
PS7, Line 747: def validateTablesMatch(tableA: String, tableB: String): Unit
= {
Does it also make sense to test restoring a table that has had its owner
changed mid-way through inserting rows?
http://gerrit.cloudera.org:8080/#/c/16126/7/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala@750
PS7, Line 750: assertEquals(tA.getOwner, tB.getOwner)
nit: maybe also check that the owner isn't empty?
--
To view, visit http://gerrit.cloudera.org:8080/16126
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143
Gerrit-Change-Number: 16126
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 08 Jul 2020 02:14:27 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3090 Support backing up ownership info
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16126 ) Change subject: KUDU-3090 Support backing up ownership info .. Patch Set 6: I think it makes sense to add the flag as an optional escape hatch for the newly required extra privilege issue without adjusting the defaults. It can and probably should be a follow on change. -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 06 Jul 2020 18:35:54 + Gerrit-HasComments: No
[kudu-CR] KUDU-3090 Support backing up ownership info
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16126 ) Change subject: KUDU-3090 Support backing up ownership info .. Patch Set 6: > Patch Set 4: Code-Review+2 > > Are you planning to add a flag to the restore job to ignore the owner on > default and restore as the user who runs the restore job? > > That would be useful now that the restore job requires potentially more > privileges than before. Tbh I'm not sure what the correct behavior would be here. It could be considered a breaking change whatever we do: 1) if we leave it as is, then higher level of privilege is required to perform the same action in the new version 2) if we add a flag to disregard owners and restore everything without setting the owner explicitly (the user running the script will be owner of every table), but we don't make it the default, then we still have the same problem, we just add a workaround. 3) if we add a flag to preserve owner but change the default behavior, then we're not actually doing what the user might expect (backing up and restoring all metadata). -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 06 Jul 2020 16:18:03 + Gerrit-HasComments: No
[kudu-CR] KUDU-3090 Support backing up ownership info
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16126 ) Change subject: KUDU-3090 Support backing up ownership info .. Patch Set 4: Code-Review+2 Are you planning to add a flag to the restore job to ignore the owner on default and restore as the user who runs the restore job? That would be useful now that the restore job requires potentially more privileges than before. -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 06 Jul 2020 13:02:55 + Gerrit-HasComments: No
[kudu-CR] KUDU-3090 Support backing up ownership info
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16126 ) Change subject: KUDU-3090 Support backing up ownership info .. Patch Set 4: Verified+1 unrelated flaky test -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 01 Jul 2020 16:43:39 + Gerrit-HasComments: No
[kudu-CR] KUDU-3090 Support backing up ownership info
Attila Bukor has removed a vote on this change. Change subject: KUDU-3090 Support backing up ownership info .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3090 Support backing up ownership info
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16126 ) Change subject: KUDU-3090 Support backing up ownership info .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/16126/3/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala File java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala: PS3: > Can you set non-default owners in these tests? yep, changed test utils to set a random owner when creating random tables (which is used by this). -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 01 Jul 2020 15:21:49 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3090 Support backing up ownership info
Hello Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16126 to look at the new patch set (#4). Change subject: KUDU-3090 Support backing up ownership info .. KUDU-3090 Support backing up ownership info Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 --- M java/kudu-backup-common/src/main/protobuf/backup.proto M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala M java/kudu-client/src/main/java/org/apache/kudu/util/SchemaGenerator.java 4 files changed, 9 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/16126/4 -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3090 Support backing up ownership info
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16126 ) Change subject: KUDU-3090 Support backing up ownership info .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/16126/3/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala File java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala: PS3: Can you set non-default owners in these tests? -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 3 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 01 Jul 2020 05:35:18 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3090 Support backing up ownership info
Attila Bukor has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16126 Change subject: KUDU-3090 Support backing up ownership info .. KUDU-3090 Support backing up ownership info Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 --- M java/kudu-backup-common/src/main/protobuf/backup.proto M java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala M java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala 3 files changed, 7 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/16126/1 -- To view, visit http://gerrit.cloudera.org:8080/16126 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I963db0a36cd4b7f080944ed46fc4119b1e055143 Gerrit-Change-Number: 16126 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor
