[jira] [Comment Edited] (HADOOP-13914) s3guard: improve S3AFileStatus#isEmptyDirectory handling

2017-03-09 Thread Sean Mackrory (JIRA)

[ 
https://issues.apache.org/jira/browse/HADOOP-13914?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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 @@
   
 
   
+  
+  
+
+
+
+  
 
{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 @@
   
 
   
+  
+  
+
+
+
+  
 
{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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Comment Edited] (HADOOP-13914) s3guard: improve S3AFileStatus#isEmptyDirectory handling

2017-03-07 Thread Aaron Fabbri (JIRA)

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

Aaron Fabbri edited comment on HADOOP-13914 at 3/8/17 1:26 AM:
---

Attaching v6 patch.

Changes from v5:

- Use wantEmptyDirectoryFlag to avoid extra DDB lookup if the caller doesn't 
care.  Changed two spots mentioned by [~liuml07] and [~rajesh.balamohan].
- Make empty dir test assert messages more descriptive
- Add helper function for creating FileStatus in DynamoDBMetadataStore

Testing: Ran all integration and unit tests with and without {{-Ds3guard 
-Ddynamo}} in us-west-2


was (Author: fabbri):
Attaching v6 patch.

Changes from v5:

- Use wantEmptyDirectoryFlag to avoid extra DDB lookup if the caller doesn't 
care.  Changed two spots mentioned by [~liuml07] and [~rajesh.balamohan].
- Make empty dir test assert messages more descriptive
- Add helper function for creating FileStatus in DynamoDBMetadataStore

> 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, 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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org