You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/08/17 15:17:29 UTC
[GitHub] [ozone] ayushtkn opened a new pull request, #3687: HDDS-7132. GetFileStatus returns NULL for some paths.
ayushtkn opened a new pull request, #3687:
URL: https://github.com/apache/ozone/pull/3687
## What changes were proposed in this pull request?
Rather than eating up the exception & returning NULL for GetFileStatus, throw the exception.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7132
## How was this patch tested?
Added a UnitTest
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #3687: HDDS-7132. GetFileStatus returns NULL for some paths.
Posted by GitBox <gi...@apache.org>.
adoroszlai merged PR #3687:
URL: https://github.com/apache/ozone/pull/3687
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a diff in pull request #3687: HDDS-7132. GetFileStatus returns NULL for some paths.
Posted by GitBox <gi...@apache.org>.
kerneltime commented on code in PR #3687:
URL: https://github.com/apache/ozone/pull/3687#discussion_r950558040
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -759,6 +759,8 @@ public FileStatus getFileStatus(Path f) throws IOException {
if (ex.getResult().equals(OMException.ResultCodes.KEY_NOT_FOUND)) {
throw new FileNotFoundException("File not found. path:" + f);
}
+ LOG.warn("GetFileStatus failed for path {}", f, ex);
Review Comment:
To be consistent with the signature for this interface, `catch (IOException ex)` instead of `catch (OMException ex)`.
I am not sure if the underlying implementation is guaranteed to always throw `OMException`. For other non `IOExceptions` exceptions, it might be better to map others as to `IOException` as well.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #3687: HDDS-7132. GetFileStatus returns NULL for some paths.
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3687:
URL: https://github.com/apache/ozone/pull/3687#issuecomment-1227204205
Thanks @ayushtkn for the patch, @kerneltime, @myskov, @smengcl for the review.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ayushtkn commented on a diff in pull request #3687: HDDS-7132. GetFileStatus returns NULL for some paths.
Posted by GitBox <gi...@apache.org>.
ayushtkn commented on code in PR #3687:
URL: https://github.com/apache/ozone/pull/3687#discussion_r950713159
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -759,6 +759,8 @@ public FileStatus getFileStatus(Path f) throws IOException {
if (ex.getResult().equals(OMException.ResultCodes.KEY_NOT_FOUND)) {
throw new FileNotFoundException("File not found. path:" + f);
}
+ LOG.warn("GetFileStatus failed for path {}", f, ex);
Review Comment:
Changed OME to IOE
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ayushtkn commented on a diff in pull request #3687: HDDS-7132. GetFileStatus returns NULL for some paths.
Posted by GitBox <gi...@apache.org>.
ayushtkn commented on code in PR #3687:
URL: https://github.com/apache/ozone/pull/3687#discussion_r949731063
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -759,6 +759,8 @@ public FileStatus getFileStatus(Path f) throws IOException {
if (ex.getResult().equals(OMException.ResultCodes.KEY_NOT_FOUND)) {
throw new FileNotFoundException("File not found. path:" + f);
}
+ LOG.debug("GetFileStatus failed for path {}", f, ex);
Review Comment:
makes sense, I will change it to WARN
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] myskov commented on a diff in pull request #3687: HDDS-7132. GetFileStatus returns NULL for some paths.
Posted by GitBox <gi...@apache.org>.
myskov commented on code in PR #3687:
URL: https://github.com/apache/ozone/pull/3687#discussion_r948978834
##########
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java:
##########
@@ -759,6 +759,8 @@ public FileStatus getFileStatus(Path f) throws IOException {
if (ex.getResult().equals(OMException.ResultCodes.KEY_NOT_FOUND)) {
throw new FileNotFoundException("File not found. path:" + f);
}
+ LOG.debug("GetFileStatus failed for path {}", f, ex);
Review Comment:
perhaps error or warn log level is better than debug for this kind of situation?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] ayushtkn commented on pull request #3687: HDDS-7132. GetFileStatus returns NULL for some paths.
Posted by GitBox <gi...@apache.org>.
ayushtkn commented on PR #3687:
URL: https://github.com/apache/ozone/pull/3687#issuecomment-1226859881
Thanx @smengcl , my bad seems those files got auto generated and got into my commit, I have removed them
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org