smengcl commented on a change in pull request #906:
URL: https://github.com/apache/hadoop-ozone/pull/906#discussion_r425397121
##########
File path:
hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java
##########
@@ -469,15 +478,72 @@ public boolean delete(Path f, boolean recursive) throws
IOException {
return false;
}
+ OFSPath ofsPath = new OFSPath(key);
+
+ // Handle rm root
+ if (ofsPath.isRoot()) {
+ // Intentionally drop support for rm root
+ // because it is too dangerous and doesn't provide much value
+ LOG.warn("delete: OFS does not support rm root. "
+ + "To wipe the cluster, please re-init OM instead.");
+ return false;
+ }
+
+ // Handle delete volume
+ if (ofsPath.isVolume()) {
+ String volumeName = ofsPath.getVolumeName();
+ if (recursive) {
+ // Delete all buckets first
+ OzoneVolume volume =
+ adapterImpl.getObjectStore().getVolume(volumeName);
+ Iterator<? extends OzoneBucket> it = volume.listBuckets("");
+ while (it.hasNext()) {
+ OzoneBucket bucket = it.next();
+ String nextBucket = addTrailingSlashIfNeeded(
+ f.toString()) + bucket.getName();
+ delete(new Path(nextBucket), true);
+ }
+ }
+ try {
+ adapterImpl.getObjectStore().deleteVolume(volumeName);
+ return true;
+ } catch (OMException ex) {
+ // volume is not empty
+ if (ex.getResult() == VOLUME_NOT_EMPTY) {
+ throw new PathIsNotEmptyDirectoryException(f.toString());
+ } else {
+ throw ex;
+ }
+ }
+ }
+
result = innerDelete(f, recursive);
+
+ // Handle delete bucket
+ if (ofsPath.isBucket()) {
+ OzoneVolume volume =
+ adapterImpl.getObjectStore().getVolume(ofsPath.getVolumeName());
+ try {
+ volume.deleteBucket(ofsPath.getBucketName());
+ return result;
+ } catch (OMException ex) {
+ // bucket is not empty
+ if (ex.getResult() == BUCKET_NOT_EMPTY) {
+ throw new PathIsNotEmptyDirectoryException(f.toString());
+ } else {
+ throw ex;
+ }
+ }
+ }
+
} else {
LOG.debug("delete: Path is a file: {}", f);
result = adapter.deleteObject(key);
}
if (result) {
// If this delete operation removes all files/directories from the
- // parent direcotry, then an empty parent directory must be created.
+ // parent directory, then an empty parent directory must be created.
Review comment:
as far as I recall, this should relate to the fact that ozone doesn't
create all dirs along the way if we create a key inside a deep directory. e.g.
if we create `/volume1/bucket1/dir1/subdir1/key1`, key`/volume1/bucket1/dir1`
and `/volume1/bucket1/dir1/subdir1` will NOT be actually created (unless we
explicitly create them). This behavior breaks some iteration logic on OM side
(I couldn't remember exactly which at the moment).
@hanishakoneru should have more insights into this.
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org
