You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/03/10 19:07:44 UTC
[GitHub] [accumulo] milleruntime opened a new pull request #1558: Resolve
relative delete markers on upgrade. Closes #1463
milleruntime opened a new pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558
* Also some minor clean up to Upgrader9to10
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1558: Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r390943068
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -430,26 +432,27 @@ public void upgradeFileDeletes(ServerContext ctx, Ample.DataLevel level) {
}
}
+ /**
+ * If path of file to delete is a directory, change it to all volumes. See {@link GcVolumeUtil}.
+ * For example: A directory "hdfs://localhost:9000/accumulo/tables/5a/t-0005" with depth = 4 will
+ * be switched to "agcav:/tables/5a/t-0005". A file
+ * "hdfs://localhost:9000/accumulo/tables/5a/t-0005/A0012.rf" with depth = 5 will be returned as
+ * is.
+ */
@VisibleForTesting
- static String switchToAllVolumes(String olddelete) {
- Path relPath = VolumeManager.FileType.TABLE.removeVolume(new Path(olddelete));
-
- if (relPath == null) {
- // An old style relative delete marker of the form /<table id>/<tablet dir>[/<file>]
- relPath = new Path("/" + VolumeManager.FileType.TABLE.getDirectory() + olddelete);
- Preconditions.checkState(
- olddelete.startsWith("/") && relPath.depth() == 3 || relPath.depth() == 4,
- "Unrecongnized relative delete marker {}", olddelete);
- }
-
- if (relPath.depth() == 3 && !relPath.getName().startsWith(Constants.BULK_PREFIX)) {
- return GcVolumeUtil.getDeleteTabletOnAllVolumesUri(TableId.of(relPath.getParent().getName()),
- relPath.getName());
+ static String switchToAllVolumes(Path olddelete) {
+ // for directory, change volume to all volumes
+ if (olddelete.depth() == 4 && !olddelete.getName().startsWith(Constants.BULK_PREFIX)) {
Review comment:
Ah OK. Then we still need to remove the volume before checking.
@ctubbsii I added a comment above the line and at the beginning of the method. I will add the ```removeVolume``` call back and update the comments.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1558: Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r390944022
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -625,4 +626,39 @@ private static Path resolveRelativePath(String metadataEntry, Key key) {
return new Path(prefix + tableId.canonical() + metadataEntry);
}
}
+
+ /**
+ * Resolve old relative delete markers of the form /tableId/tabletDir/[file] to
+ * UpgradeVolume/tables/tableId/tabletDir/[file]
+ */
+ static Path resolveRelativeDelete(VolumeManager fs, String oldDelete, String upgradeProperty) {
+ Path pathNoVolume = VolumeManager.FileType.TABLE.removeVolume(new Path(oldDelete));
+
+ // abs path won't be null so return
+ if (pathNoVolume != null)
+ return pathNoVolume;
Review comment:
The ```removeVolume``` method will return null if it doesn't have a volume aka it is a relative path.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1558: Resolve relative
delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#issuecomment-598298548
@ctubbsii I think this is good to go if you are happy with updated the comments
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1558: Resolve relative
delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#issuecomment-598759883
I updated the comment again and added test for this scenario to ```FileTypeTest.``` I get a sense that you are more concerned about the FileType method itself then the upgrade code. There is still potential for the bug you created [here](https://issues.apache.org/jira/browse/ACCUMULO-3577) which I think should be addressed separately.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1558: Resolve relative
delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#issuecomment-599843404
To aid future code archaeologists. The references to #1463 in the commits here are a mistake. They should reference #1465 instead. :smiley_cat:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1558: Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r390944429
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -625,4 +626,39 @@ private static Path resolveRelativePath(String metadataEntry, Key key) {
return new Path(prefix + tableId.canonical() + metadataEntry);
}
}
+
+ /**
+ * Resolve old relative delete markers of the form /tableId/tabletDir/[file] to
+ * UpgradeVolume/tables/tableId/tabletDir/[file]
+ */
+ static Path resolveRelativeDelete(VolumeManager fs, String oldDelete, String upgradeProperty) {
+ Path pathNoVolume = VolumeManager.FileType.TABLE.removeVolume(new Path(oldDelete));
+
+ // abs path won't be null so return
+ if (pathNoVolume != null)
+ return pathNoVolume;
+
+ // use Path to check the format is correct
+ Path pathToCheck = new Path(oldDelete);
+ Preconditions.checkState(
+ oldDelete.startsWith("/") && (pathToCheck.depth() == 2 || pathToCheck.depth() == 3),
+ "Unrecognized relative delete marker {}", oldDelete);
+
+ // found relative paths so verify the property used to build the absolute paths
+ if (upgradeProperty == null || upgradeProperty.isBlank()) {
+ throw new IllegalArgumentException(
+ "Missing required property " + Property.INSTANCE_VOLUMES_UPGRADE_RELATIVE.getKey());
+ }
+ Path absPath =
+ new Path(upgradeProperty, VolumeManager.FileType.TABLE.getDirectory() + oldDelete);
+ try {
+ if (!fs.exists(absPath)) {
Review comment:
Good to know, thanks. I will remove this check.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii merged pull request #1558: Resolve relative
delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1558:
Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r391971048
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -625,4 +626,39 @@ private static Path resolveRelativePath(String metadataEntry, Key key) {
return new Path(prefix + tableId.canonical() + metadataEntry);
}
}
+
+ /**
+ * Resolve old relative delete markers of the form /tableId/tabletDir/[file] to
+ * UpgradeVolume/tables/tableId/tabletDir/[file]
+ */
+ static Path resolveRelativeDelete(VolumeManager fs, String oldDelete, String upgradeProperty) {
+ Path pathNoVolume = VolumeManager.FileType.TABLE.removeVolume(new Path(oldDelete));
+
+ // abs path won't be null so return
+ if (pathNoVolume != null)
+ return pathNoVolume;
+
+ // use Path to check the format is correct
+ Path pathToCheck = new Path(oldDelete);
+ Preconditions.checkState(
+ oldDelete.startsWith("/") && (pathToCheck.depth() == 2 || pathToCheck.depth() == 3),
Review comment:
This is definitely better. Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1558:
Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r390638155
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -625,4 +626,39 @@ private static Path resolveRelativePath(String metadataEntry, Key key) {
return new Path(prefix + tableId.canonical() + metadataEntry);
}
}
+
+ /**
+ * Resolve old relative delete markers of the form /tableId/tabletDir/[file] to
+ * UpgradeVolume/tables/tableId/tabletDir/[file]
+ */
+ static Path resolveRelativeDelete(VolumeManager fs, String oldDelete, String upgradeProperty) {
+ Path pathNoVolume = VolumeManager.FileType.TABLE.removeVolume(new Path(oldDelete));
+
+ // abs path won't be null so return
+ if (pathNoVolume != null)
+ return pathNoVolume;
Review comment:
This says "absolute path won't be null", but that's not the same as "relative path must be null". Can relative path be non-null? If so, then this will return for both absolute paths and non-null relative paths.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1558:
Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r390637358
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -625,4 +626,39 @@ private static Path resolveRelativePath(String metadataEntry, Key key) {
return new Path(prefix + tableId.canonical() + metadataEntry);
}
}
+
+ /**
+ * Resolve old relative delete markers of the form /tableId/tabletDir/[file] to
+ * UpgradeVolume/tables/tableId/tabletDir/[file]
+ */
+ static Path resolveRelativeDelete(VolumeManager fs, String oldDelete, String upgradeProperty) {
+ Path pathNoVolume = VolumeManager.FileType.TABLE.removeVolume(new Path(oldDelete));
+
+ // abs path won't be null so return
+ if (pathNoVolume != null)
+ return pathNoVolume;
+
+ // use Path to check the format is correct
+ Path pathToCheck = new Path(oldDelete);
+ Preconditions.checkState(
+ oldDelete.startsWith("/") && (pathToCheck.depth() == 2 || pathToCheck.depth() == 3),
Review comment:
Should have comment to briefly explain intent of this check, so its implementation can be more easily verified. The current comment just says "check the format is correct", but does not explain what it means to be "correct".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on a change in pull request
#1558: Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r390670901
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -625,4 +626,39 @@ private static Path resolveRelativePath(String metadataEntry, Key key) {
return new Path(prefix + tableId.canonical() + metadataEntry);
}
}
+
+ /**
+ * Resolve old relative delete markers of the form /tableId/tabletDir/[file] to
+ * UpgradeVolume/tables/tableId/tabletDir/[file]
+ */
+ static Path resolveRelativeDelete(VolumeManager fs, String oldDelete, String upgradeProperty) {
+ Path pathNoVolume = VolumeManager.FileType.TABLE.removeVolume(new Path(oldDelete));
+
+ // abs path won't be null so return
+ if (pathNoVolume != null)
+ return pathNoVolume;
+
+ // use Path to check the format is correct
+ Path pathToCheck = new Path(oldDelete);
+ Preconditions.checkState(
+ oldDelete.startsWith("/") && (pathToCheck.depth() == 2 || pathToCheck.depth() == 3),
+ "Unrecognized relative delete marker {}", oldDelete);
+
+ // found relative paths so verify the property used to build the absolute paths
+ if (upgradeProperty == null || upgradeProperty.isBlank()) {
+ throw new IllegalArgumentException(
+ "Missing required property " + Property.INSTANCE_VOLUMES_UPGRADE_RELATIVE.getKey());
+ }
+ Path absPath =
+ new Path(upgradeProperty, VolumeManager.FileType.TABLE.getDirectory() + oldDelete);
+ try {
+ if (!fs.exists(absPath)) {
Review comment:
Delete entries do not have to exists. This can happen when the GC deletes some files, but the GC process dies before it deletes the delete entries.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1558: Resolve relative
delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#issuecomment-598881148
The "otherwise" wording makes it clear this is a bi-implication. :smiley_cat:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on a change in pull request
#1558: Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r390670107
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -430,26 +432,27 @@ public void upgradeFileDeletes(ServerContext ctx, Ample.DataLevel level) {
}
}
+ /**
+ * If path of file to delete is a directory, change it to all volumes. See {@link GcVolumeUtil}.
+ * For example: A directory "hdfs://localhost:9000/accumulo/tables/5a/t-0005" with depth = 4 will
+ * be switched to "agcav:/tables/5a/t-0005". A file
+ * "hdfs://localhost:9000/accumulo/tables/5a/t-0005/A0012.rf" with depth = 5 will be returned as
+ * is.
+ */
@VisibleForTesting
- static String switchToAllVolumes(String olddelete) {
- Path relPath = VolumeManager.FileType.TABLE.removeVolume(new Path(olddelete));
-
- if (relPath == null) {
- // An old style relative delete marker of the form /<table id>/<tablet dir>[/<file>]
- relPath = new Path("/" + VolumeManager.FileType.TABLE.getDirectory() + olddelete);
- Preconditions.checkState(
- olddelete.startsWith("/") && relPath.depth() == 3 || relPath.depth() == 4,
- "Unrecongnized relative delete marker {}", olddelete);
- }
-
- if (relPath.depth() == 3 && !relPath.getName().startsWith(Constants.BULK_PREFIX)) {
- return GcVolumeUtil.getDeleteTabletOnAllVolumesUri(TableId.of(relPath.getParent().getName()),
- relPath.getName());
+ static String switchToAllVolumes(Path olddelete) {
+ // for directory, change volume to all volumes
+ if (olddelete.depth() == 4 && !olddelete.getName().startsWith(Constants.BULK_PREFIX)) {
Review comment:
Volumes do not have to be depth of 1. For example a user could configure a volume like `hdfs://localhost:9000/dev/accumulo`. This would make a dir using that volume have a depth != 4.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1558:
Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r391970913
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -625,4 +630,30 @@ private static Path resolveRelativePath(String metadataEntry, Key key) {
return new Path(prefix + tableId.canonical() + metadataEntry);
}
}
+
+ /**
+ * Resolve old relative delete markers of the form /tableId/tabletDir/[file] to
+ * UpgradeVolume/tables/tableId/tabletDir/[file]
+ */
+ static Path resolveRelativeDelete(String oldDelete, String upgradeProperty) {
+ Path pathNoVolume = VolumeManager.FileType.TABLE.removeVolume(new Path(oldDelete));
+
+ // removeVolume will return null if path doesn't have a volume aka is a relative path
Review comment:
I'm still a bit concerned about the wording of this comment here. Is this an `if` or an `iff (if and only if)`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1558:
Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r390635122
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -430,26 +432,27 @@ public void upgradeFileDeletes(ServerContext ctx, Ample.DataLevel level) {
}
}
+ /**
+ * If path of file to delete is a directory, change it to all volumes. See {@link GcVolumeUtil}.
+ * For example: A directory "hdfs://localhost:9000/accumulo/tables/5a/t-0005" with depth = 4 will
+ * be switched to "agcav:/tables/5a/t-0005". A file
+ * "hdfs://localhost:9000/accumulo/tables/5a/t-0005/A0012.rf" with depth = 5 will be returned as
+ * is.
+ */
@VisibleForTesting
- static String switchToAllVolumes(String olddelete) {
- Path relPath = VolumeManager.FileType.TABLE.removeVolume(new Path(olddelete));
-
- if (relPath == null) {
- // An old style relative delete marker of the form /<table id>/<tablet dir>[/<file>]
- relPath = new Path("/" + VolumeManager.FileType.TABLE.getDirectory() + olddelete);
- Preconditions.checkState(
- olddelete.startsWith("/") && relPath.depth() == 3 || relPath.depth() == 4,
- "Unrecongnized relative delete marker {}", olddelete);
- }
-
- if (relPath.depth() == 3 && !relPath.getName().startsWith(Constants.BULK_PREFIX)) {
- return GcVolumeUtil.getDeleteTabletOnAllVolumesUri(TableId.of(relPath.getParent().getName()),
- relPath.getName());
+ static String switchToAllVolumes(Path olddelete) {
+ // for directory, change volume to all volumes
+ if (olddelete.depth() == 4 && !olddelete.getName().startsWith(Constants.BULK_PREFIX)) {
Review comment:
What is the significance of depth 4? Could add to a comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on a change in pull request
#1558: Resolve relative delete markers on upgrade. Closes #1463
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1558: Resolve relative delete markers on upgrade. Closes #1463
URL: https://github.com/apache/accumulo/pull/1558#discussion_r390964036
##########
File path: server/master/src/main/java/org/apache/accumulo/master/upgrade/Upgrader9to10.java
##########
@@ -625,4 +626,39 @@ private static Path resolveRelativePath(String metadataEntry, Key key) {
return new Path(prefix + tableId.canonical() + metadataEntry);
}
}
+
+ /**
+ * Resolve old relative delete markers of the form /tableId/tabletDir/[file] to
+ * UpgradeVolume/tables/tableId/tabletDir/[file]
+ */
+ static Path resolveRelativeDelete(VolumeManager fs, String oldDelete, String upgradeProperty) {
+ Path pathNoVolume = VolumeManager.FileType.TABLE.removeVolume(new Path(oldDelete));
+
+ // abs path won't be null so return
+ if (pathNoVolume != null)
+ return pathNoVolume;
+
+ // use Path to check the format is correct
+ Path pathToCheck = new Path(oldDelete);
+ Preconditions.checkState(
+ oldDelete.startsWith("/") && (pathToCheck.depth() == 2 || pathToCheck.depth() == 3),
Review comment:
Updated comments. Let me know if it is more clear now
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services