ashishkumar50 commented on code in PR #5580:
URL: https://github.com/apache/ozone/pull/5580#discussion_r1401758519
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -639,30 +666,25 @@ OzoneListingIterator getDeleteIterator()
* @return true if successfully deletes all required keys, false otherwise
* @throws IOException
*/
- private boolean innerDelete(Path f, boolean recursive) throws IOException {
+ private InnerDeleteResult innerDelete(Path f, boolean recursive)
+ throws IOException {
LOG.trace("delete() path:{} recursive:{}", f, recursive);
try {
OzoneListingIterator iterator =
new DeleteIteratorFactory(f, recursive).getDeleteIterator();
- return iterator.iterate();
+ boolean success = iterator.iterate();
+ boolean partiallyDeleted = iterator.isPartiallyComplete();
+ return new InnerDeleteResult(success, partiallyDeleted);
} catch (FileNotFoundException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Couldn't delete {} - does not exist", f);
}
- return false;
+ return new InnerDeleteResult(false, false);
}
}
-
- /**
- * {@inheritDoc}
- *
- * OFS supports volume and bucket deletion, recursive or non-recursive.
- * e.g. delete(new Path("/volume1"), true)
- * But root deletion is explicitly disallowed for safety concerns.
- */
- @Override
- public boolean delete(Path f, boolean recursive) throws IOException {
+ public InnerDeleteResult getDeleteResult(Path f, boolean recursive)
Review Comment:
Better to change the method name as currently it sounds like it just returns
delete result. But actually it's deleting and returning the delete response.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneFsDelete.java:
##########
@@ -139,10 +140,22 @@ protected void processPath(PathData item) throws
IOException {
if (moveToTrash(item) || !canBeSafelyDeleted(item)) {
return;
}
- if (!item.fs.delete(path, deleteDirs)) {
- throw new PathIOException(item.toString());
+ BasicRootedOzoneFileSystem.InnerDeleteResult innerDeleteResult = null;
+ if (item.fs.getUri().getScheme().equals(OZONE_OFS_URI_SCHEME)) {
+ BasicRootedOzoneFileSystem ofs = (BasicRootedOzoneFileSystem) item.fs;
+ innerDeleteResult = ofs.getDeleteResult(path, deleteDirs);
+ } else {
+ if (!item.fs.delete(path, deleteDirs)) {
+ throw new PathIOException(item.toString());
+ }
+ }
+ if (innerDeleteResult != null && innerDeleteResult.isPartiallyDeleted())
{
Review Comment:
Need to consider `innerDeleteResult.isSuccess()` as `false` also and throw
exception to keep the behaviour same as before.
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneFileSystem.java:
##########
@@ -562,7 +583,13 @@ boolean processKeyPath(List<String> keyPathList) {
LOG.trace("Deleting keys: {}", keyPathList);
boolean succeed = adapterImpl.deleteObjects(this.bucket, keyPathList);
// if recursive delete is requested ignore the return value of
- // deleteObject and issue deletes for other keys.
+ // deleteObject and issue deletes for other keys and set partiallyDeleted
+ // to true
+ if (recursive && !succeed) {
+ if (!isPartiallyComplete()) {
Review Comment:
`if (!isPartiallyComplete())` check not required as we want to make
PartiallyComplete as true in any case.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
