hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Mackrory (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HADOOP-13914) s3guard: improve S3AFileStatus#isEmptyDirectory handling
Date Thu, 09 Mar 2017 19:27:38 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-13914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15903685#comment-15903685
] 

Sean Mackrory edited comment on HADOOP-13914 at 3/9/17 7:26 PM:
----------------------------------------------------------------

I reviewed the .006. patch, and I'm a +1 on the code itself.

hadoop-common unit test results we've seen before and aren't related. There's still that checkstyle
issue with innerRename - I'm of the opinion that we should try and refactor that to fit under
150, but after this is merged to trunk. That's probably one of the tricker things we'll have
to merge and refactoring it that way will only make that worse. Anyone disagree?

We can add the following to ignore the findbugs issue:
{code}
diff --git a/hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml b/hadoop-tools/hadoop-aws/dev-support/findbugs
-exclude.xml
index ffb0a79..3464e71 100644
--- a/hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml
+++ b/hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml
@@ -26,4 +26,10 @@
   <Match>
     <Class name="org.apache.hadoop.fs.s3.INode" />
   </Match>
+  <!-- Redunant null check makes intent of code clearer, and does no harm. -->
+  <Match>
+    <Class name="org.apache.hadoop.fs.s3a.S3AFileSystem" />
+    <Method name="s3Exists" />
+    <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE" />
+  </Match>
 </FindBugsFilter>
{code}


was (Author: mackrorysd):
I reviewed the .006. patch, and I'm a +1 on the code itself.

hadoop-common unit test results we've seen before and aren't related. There's still that checkstyle
issue with innerRename - I'm of the opinion that we should try and refactor that to fit under
150, but after this is merged to trunk. That's probably one of the tricker things we'll have
to merge and refactoring it that way will only make that worse. Anyone disagree?

We can add the following to ignore the findbugs issue:
{quote}
diff --git a/hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml b/hadoop-tools/hadoop-aws/dev-support/findbugs
-exclude.xml
index ffb0a79..3464e71 100644
--- a/hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml
+++ b/hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml
@@ -26,4 +26,10 @@
   <Match>
     <Class name="org.apache.hadoop.fs.s3.INode" />
   </Match>
+  <!-- Redunant null check makes intent of code clearer, and does no harm. -->
+  <Match>
+    <Class name="org.apache.hadoop.fs.s3a.S3AFileSystem" />
+    <Method name="s3Exists" />
+    <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE" />
+  </Match>
 </FindBugsFilter>
{quote}

> s3guard: improve S3AFileStatus#isEmptyDirectory handling
> --------------------------------------------------------
>
>                 Key: HADOOP-13914
>                 URL: https://issues.apache.org/jira/browse/HADOOP-13914
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/s3
>    Affects Versions: HADOOP-13345
>            Reporter: Aaron Fabbri
>            Assignee: Aaron Fabbri
>         Attachments: HADOOP-13914-HADOOP-13345.000.patch, HADOOP-13914-HADOOP-13345.002.patch,
HADOOP-13914-HADOOP-13345.003.patch, HADOOP-13914-HADOOP-13345.004.patch, HADOOP-13914-HADOOP-13345.005.patch,
HADOOP-13914-HADOOP-13345.006.patch, HADOOP-13914-HADOOP-13345.007.patch, s3guard-empty-dirs.md,
test-only-HADOOP-13914.patch
>
>
> As discussed in HADOOP-13449, proper support for the isEmptyDirectory() flag stored in
S3AFileStatus is missing from DynamoDBMetadataStore.
> The approach taken by LocalMetadataStore is not suitable for the DynamoDB implementation,
and also sacrifices good code separation to minimize S3AFileSystem changes pre-merge to trunk.
> I will attach a design doc that attempts to clearly explain the problem and preferred
solution.  I suggest we do this work after merging the HADOOP-13345 branch to trunk, but am
open to suggestions.
> I can also attach a patch of a integration test that exercises the missing case and demonstrates
a failure with DynamoDBMetadataStore.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Mime
View raw message