Mukul Kumar Singh created HDFS-12768:
----------------------------------------

             Summary: Ozone: Qualify OzoneFileSystem paths in Filesystem APIs
                 Key: HDFS-12768
                 URL: https://issues.apache.org/jira/browse/HDFS-12768
             Project: Hadoop HDFS
          Issue Type: Sub-task
          Components: ozone
    Affects Versions: HDFS-7240
            Reporter: Mukul Kumar Singh
            Assignee: Mukul Kumar Singh
            Priority: Major
             Fix For: HDFS-7240


This is based on [~ste...@apache.org]'s comments on HDFS-7240. This jira will 
be used to qualify filesystem paths before they are used, this jira will also 
address rest of the review comments for filesystem api as well.

General
1) various places use LOG.info("text " + something). they should all move to 
LOG.info("text {}", something)
2) Once OzoneException -> IOE, you can cut the catch and translate here.
3) qualify path before all uses. That's needed to stop them being relative, and 
to catch things like someone calling ozfs.rename("o3://bucket/src", 
"s3a://bucket/dest"), delete("s3a://bucket/path"), etc, as well as problems 
with validation happening before paths are made absolute.
4) RenameIterator.iterate() it's going to log @ warn whenever it can't delete a 
temp file because it doesn't exist, which may be a distraction in failures. 
Better: if(!tmpFile.delete() && tmpFile.exists()), as that will only warn if 
the temp file is actually there.

OzoneFileSystem.rename().
1) Qualify all the paths before doing directory validation. Otherwise you can 
defeat the "don't rename into self checks" rename("/path/src", 
"/path/../path/src/dest").
2) Log @ debu all the paths taken before returning so you can debug if needed.
3) S3A rename ended up having a special RenameFailedException() which 
innerRename() raises, with text and return code. Outer rename logs the text and 
returns the return code. This means that all failing paths have an exception 
clearly thrown, and when we eventually make rename/3 public, it's lined up to 
throw exceptions back to the caller. Consider copying this code.

OzoneFileSystem.delete
1) qualify path before use
 2) dont' log at error if you can't delete a nonexistent path, it is used 
everywhere for silent cleanup. Cut it

OzoneFileSystem.ListStatusIterator
1) make status field final

OzoneFileSystem.mkdir
1) do qualify path first.

OzoneFileSystem.getFileStatus
1) getKeyInfo() catches all exceptions and maps to null, which is interpreted 
not found and eventually surfaces as FNFE. This is misleading if the failure is 
for any other reason.
2) Once OzoneException -> IOException, getKeyInfo() should only catch & 
downgrade the explicit not found (404?) responses.

OzoneFileSystem.listKeys()
unless this needs to be tagged as VisibleForTesting, make private. 

OzoneFileSystem.getDefaultBlockSize()
implement getDefaultBlockSize(); add a config option to let people set it. add 
a sensible default like 64 or 128 MB.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to