kudu-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adar Dembo (Code Review)" <ger...@cloudera.org>
Subject [kudu-CR] [backup] Support table name alterations between Kudu backups
Date Thu, 02 May 2019 00:55:38 GMT
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13210 )

Change subject: [backup] Support table name alterations between Kudu backups

Patch Set 2:


File java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduBackup.scala:

PS1, Line 51:         // the static table ID even when the table name changes between backups.
            :         val tableIds = options.tables.map { tableName =>
            :           val table = context.syncClient.openTable(tableName)
> context.syncClient.getTablesList.getTablesList returns a list of tableNames
If we're comfortable exposing the table ID in KuduTable, we should be comfortable exposing
it in getTablesList too. My guess is it was just an oversight.

File java/kudu-backup/src/main/scala/org/apache/kudu/backup/SessionIO.scala:

> You mean that sometimes I list the params and other times I don't? Do you h
I'd just prefer more consistency. I don't care whether it's Javadoc or not, but if I had to
make changes to this file, I wouldn't know whether to use Javadoc or not for the documentation.

PS1, Line 108:     val tableName = URLEncoder.encode(table.getName, "UTF-8")
> Yes, this was added in a previous patch. If you remove this table names lik
Looks like colon specifically is the issue: https://hadoop.apache.org/docs/r2.5.2/hadoop-project-dist/hadoop-common/filesystem/model.html
What do you think about escaping/unescaping it? I found and read through https://issues.apache.org/jira/browse/HDFS-13
which was amusing.

Are URL and URI encoding the same thing? I was under the impression that the former is more
heavyweight, but maybe that's wrong. Basically I'm thinking we should minimize the number
of "surprises" for users when they do a backup and then go look at the paths they got.

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id08d1fa293d76538adc61fdfc7593b1900521e01
Gerrit-Change-Number: 13210
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <granthenke@apache.org>
Gerrit-Reviewer: Adar Dembo <adar@cloudera.com>
Gerrit-Reviewer: Grant Henke <granthenke@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mpercy@apache.org>
Gerrit-Comment-Date: Thu, 02 May 2019 00:55:38 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message