[jira] [Commented] (HADOOP-13573) S3Guard: create basic contract tests for MetadataStore implementations
[
https://issues.apache.org/jira/browse/HADOOP-13573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15505228#comment-15505228
]
Hadoop QA commented on HADOOP-13573:
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
15s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 2 new or modified test
files. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 7m
22s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
19s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
13s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
25s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
17s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
28s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
14s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
16s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
16s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
10s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
22s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
13s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
31s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
11s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m
20s{color} | {color:green} hadoop-aws in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
16s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 13m 24s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Issue | HADOOP-13573 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12829306/HADOOP-13573-HADOOP-13345.003.patch
|
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
| uname | Linux d5ba93213779 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18
21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | HADOOP-13345 / 18f1f68 |
| Default Java | 1.8.0_101 |
| findbugs | v3.0.0 |
| Test Results |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10546/testReport/ |
| modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
| Console output |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10546/console |
| Powered by | Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
> S3Guard: create basic contract tests for MetadataStore implementations
> --
>
> Key: HADOOP-13573
> URL: https://issues.apache.org/jira/browse/HADOOP-13573
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
>Reporter: Aaron Fabbri
>Assignee: Aaron Fabbri
> Attachments: HADOOP-13573-HADOOP-13345.002.patch,
> HADOOP-13573-HADOOP-13345.003.patch, HADOOP-13573.001.patch
>
>
> We should have some
[jira] [Commented] (HADOOP-13573) S3Guard: create basic contract tests for MetadataStore implementations
[
https://issues.apache.org/jira/browse/HADOOP-13573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15494564#comment-15494564
]
Chris Nauroth commented on HADOOP-13573:
Thank you for the updated patch. I think we just have one more assertion that
flipped the expected and actual arguments:
{code}
assertEquals("Cached file size correct.", meta.getFileStatus().getLen(),
200);
{code}
Can you please change that?
Also, are you planning to address the TODOs now, or in a later JIRA? I'm fine
either way. Since this is a feature branch, we have freedom to leave temporary
TODOs.
> S3Guard: create basic contract tests for MetadataStore implementations
> --
>
> Key: HADOOP-13573
> URL: https://issues.apache.org/jira/browse/HADOOP-13573
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
>Reporter: Aaron Fabbri
>Assignee: Aaron Fabbri
> Attachments: HADOOP-13573-HADOOP-13345.002.patch,
> HADOOP-13573.001.patch
>
>
> We should have some contract-style unit tests for the MetadataStore interface
> to validate that the different implementations provide correct semantics.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-13573) S3Guard: create basic contract tests for MetadataStore implementations
[
https://issues.apache.org/jira/browse/HADOOP-13573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15489245#comment-15489245
]
Hadoop QA commented on HADOOP-13573:
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
18s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 2 new or modified test
files. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 14m
41s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
18s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
14s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
25s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 4m
53s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
28s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
15s{color} | {color:green} HADOOP-13345 passed {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
15s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m
15s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
10s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
20s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
13s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
31s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
11s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m
19s{color} | {color:green} hadoop-aws in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
16s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 25m 20s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Issue | HADOOP-13573 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12828377/HADOOP-13573-HADOOP-13345.002.patch
|
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
| uname | Linux 5049df43010d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed
Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | HADOOP-13345 / 18f1f68 |
| Default Java | 1.8.0_101 |
| findbugs | v3.0.0 |
| Test Results |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10502/testReport/ |
| modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
| Console output |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10502/console |
| Powered by | Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
> S3Guard: create basic contract tests for MetadataStore implementations
> --
>
> Key: HADOOP-13573
> URL: https://issues.apache.org/jira/browse/HADOOP-13573
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
>Reporter: Aaron Fabbri
>Assignee: Aaron Fabbri
> Attachments: HADOOP-13573-HADOOP-13345.002.patch,
> HADOOP-13573.001.patch
>
>
> We should have some contract-style unit tests for the
[jira] [Commented] (HADOOP-13573) S3Guard: create basic contract tests for MetadataStore implementations
[ https://issues.apache.org/jira/browse/HADOOP-13573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15488855#comment-15488855 ] Aaron Fabbri commented on HADOOP-13573: --- Thanks for the review [~cnauroth]. This is all good feedback. I don't know what I was thinking in that try-with-resources.. haha. I'll have a followup patch shortly. > S3Guard: create basic contract tests for MetadataStore implementations > -- > > Key: HADOOP-13573 > URL: https://issues.apache.org/jira/browse/HADOOP-13573 > Project: Hadoop Common > Issue Type: Sub-task > Components: fs/s3 >Reporter: Aaron Fabbri >Assignee: Aaron Fabbri > Attachments: HADOOP-13573.001.patch > > > We should have some contract-style unit tests for the MetadataStore interface > to validate that the different implementations provide correct semantics. -- This message was sent by Atlassian JIRA (v6.3.4#6332) - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-13573) S3Guard: create basic contract tests for MetadataStore implementations
[
https://issues.apache.org/jira/browse/HADOOP-13573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15488782#comment-15488782
]
Chris Nauroth commented on HADOOP-13573:
[~fabbri], thank you for the patch. The structure looks good overall. Here
are a few comments.
{code}
public AbstractMSContract() { };
{code}
I suggest removing this line, since Java will give us a default constructor
automatically.
{code}
try (MetadataStore ms = contract.getMetadataStore()) {
assertNotNull("null MetadataStore", ms);
ms.initialize(contract.getConf());
this.ms = ms;
}
{code}
I think we exit this block with the {{MetadataStore}} closed because of the
try-with-resources. It probably wasn't noticeable while testing with
{{LocalMetadataStore}}, because that has a no-op {{close}}.
{code}
assertDirectorySize("/ADirectory1/db1/", 1);
{code}
{code}
assertDirectorySize("/ADirectory1/db1/", 2);
{code}
I suggest removing trailing slashes from all path strings to avoid a potential
source of confusion. The {{Path}} constructor will normalize and drop the
trailing slashes anyway.
{code}
@Test
public void testDeleteRecursiveRoot() throws Exception {
setUpDeleteTest();
ms.deleteSubtree(new Path("/"));
{code}
I don't expect the {{FileSystem}} to call the {{MetadataStore}} this way,
because deletion of root should be rejected. For the DynamoDB implementation,
I don't think it can issue a meaningful delete for root, because root does not
have a parent, so it won't fit well in the schema. This is not a big deal
though, and we can revisit during the DynamoDB implementation.
{code}
assertEquals("File size as expected", meta.getFileStatus().getLen(), 100);
{code}
I think the "expected" and "actual" arguments were inverted in this assertion.
{code}
meta = ms.get(new Path("bollocks"));
{code}
Excellent test data! :-)
Suggestions for additional tests:
* {{put}} overwriting an existing path.
* {{delete}} a path that doesn't exist. (Does nothing, but should complete
without exception.)
* Same thing for {{deleteSubtree}}.
* Some tests already check the returned value of {{FileStatus#getLen}}. It
would be good to do the same for {{isEmptyDirectory}} (on directories) and
{{getModificationTime}} and {{getBlockSize}} (on files).
In general, assertions on values of {{FileStatus}} properties:
isEmptyDirectory, mtime, blockSize, length.
> S3Guard: create basic contract tests for MetadataStore implementations
> --
>
> Key: HADOOP-13573
> URL: https://issues.apache.org/jira/browse/HADOOP-13573
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
>Reporter: Aaron Fabbri
>Assignee: Aaron Fabbri
> Attachments: HADOOP-13573.001.patch
>
>
> We should have some contract-style unit tests for the MetadataStore interface
> to validate that the different implementations provide correct semantics.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
[jira] [Commented] (HADOOP-13573) S3Guard: create basic contract tests for MetadataStore implementations
[
https://issues.apache.org/jira/browse/HADOOP-13573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15487992#comment-15487992
]
Hadoop QA commented on HADOOP-13573:
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
18s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 2 new or modified test
files. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m
33s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m
19s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
14s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m
25s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
17s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m
33s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
16s{color} | {color:green} trunk passed {color} |
| {color:red}-1{color} | {color:red} mvninstall {color} | {color:red} 0m
18s{color} | {color:red} hadoop-aws in the patch failed. {color} |
| {color:red}-1{color} | {color:red} compile {color} | {color:red} 0m
18s{color} | {color:red} hadoop-aws in the patch failed. {color} |
| {color:red}-1{color} | {color:red} javac {color} | {color:red} 0m 18s{color}
| {color:red} hadoop-aws in the patch failed. {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
12s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} mvnsite {color} | {color:red} 0m
19s{color} | {color:red} hadoop-aws in the patch failed. {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
14s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:red}-1{color} | {color:red} findbugs {color} | {color:red} 0m
14s{color} | {color:red} hadoop-aws in the patch failed. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m
13s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 0m 19s{color}
| {color:red} hadoop-aws in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
17s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 14m 37s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Issue | HADOOP-13573 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12827942/HADOOP-13573.001.patch
|
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
| uname | Linux b0051298887d 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed
Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | trunk / db6d243 |
| Default Java | 1.8.0_101 |
| findbugs | v3.0.0 |
| mvninstall |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10497/artifact/patchprocess/patch-mvninstall-hadoop-tools_hadoop-aws.txt
|
| compile |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10497/artifact/patchprocess/patch-compile-hadoop-tools_hadoop-aws.txt
|
| javac |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10497/artifact/patchprocess/patch-compile-hadoop-tools_hadoop-aws.txt
|
| mvnsite |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10497/artifact/patchprocess/patch-mvnsite-hadoop-tools_hadoop-aws.txt
|
| findbugs |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10497/artifact/patchprocess/patch-findbugs-hadoop-tools_hadoop-aws.txt
|
| unit |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10497/artifact/patchprocess/patch-unit-hadoop-tools_hadoop-aws.txt
|
| Test Results |
https://builds.apache.org/job/PreCommit-HADOOP-Build/10497/testReport/ |
| modules | C: hadoop-tools/hadoop-aws U:
