[
https://issues.apache.org/jira/browse/HADOOP-13449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15668686#comment-15668686
]
Mingliang Liu commented on HADOOP-13449:
----------------------------------------
Thanks for your review [~fabbri]. Quick reply (will consider more carefully
before I submit a new patch this week):
# Yes, the AWS SDK version bump should be separated out. However, I found the
DynamoDBLocal is not included in version >1.11.0.1 Steve does have a patch in
[HADOOP-13050], which bumps up the version to 1.11.45. I guess we may have to
use different DynamoDBLocal version from AWS SDK (and thus DynamoDB) version.
By the way, the headache of {{jackson2}} version, I got 2.5.5 working. Next
patch will be easier to separate the version bump code out.
# I think moving {{testDescendantsIterator}} to {{MetadataStoreTestBase}} is a
very good idea. I'll try to consolidate this in next patch. It should work.
# As to the changes in the test base and local metadata store. I'm sorry for
that. I was selfish to make my test pass only. I was indeed make the test
methods in base class common/general enough so that both DDB and local MS can
use this. Plus, the local MS can override them:
{code:title=from the patch}
61
62 @Override
63 protected void verifyFileStatus(FileStatus status, long size) {
64 super.verifyFileStatus(status, size);
65
66 assertEquals("Replication value", REPLICATION,
status.getReplication());
67 assertEquals("Access time", getAccessTime(),
status.getAccessTime());
68 assertEquals("Owner", OWNER, status.getOwner());
69 assertEquals("Group", GROUP, status.getGroup());
70 assertEquals("Permission", PERMISSION, status.getPermission());
71 }
72
73 @Override
74 protected void verifyDirStatus(FileStatus status) {
75 super.verifyDirStatus(status);
76
77 assertEquals("Mod time", getModTime(),
status.getModificationTime());
78 assertEquals("Replication value", REPLICATION,
status.getReplication());
79 assertEquals("Access time", getAccessTime(),
status.getAccessTime());
80 assertEquals("Owner", OWNER, status.getOwner());
81 assertEquals("Group", GROUP, status.getGroup());
82 assertEquals("Permission", PERMISSION, status.getPermission());
83 }
{code}
Do you want me to make the base test methods untouched (as well as local), and
make DDB test override the super methods? That is also feasible, but the base
class will not be general/common enough; plus we may have duplicate code.
# {{public abstract MetadataStore getMs();}} in {{AbstractMSContract}} was
unexpected. I must have been fooled by my IDE.
# No I'll finish my review (sorry for the delay) for [HADOOP-13651] before I
run any integration/contract tests. We may have a few of follow-up JIRAs to
make end2end tests work.
The comments are driving this patch in the right direction. But.... did I
promise too much about the next version patch? I may post a mid-way patch for
quick feedback.
My final concern is that, the MetadataStore assumes all ancestor directories
(including direct parent directory) have been pre-created by the caller/user. I
have to change the base test MetadataStoreTestBase to make all the tests pass.
We have to change LocalMetadataStore accordingly. See my above comments.
> S3Guard: Implement DynamoDBMetadataStore.
> -----------------------------------------
>
> Key: HADOOP-13449
> URL: https://issues.apache.org/jira/browse/HADOOP-13449
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Reporter: Chris Nauroth
> Assignee: Mingliang Liu
> Attachments: HADOOP-13449-HADOOP-13345.000.patch,
> HADOOP-13449-HADOOP-13345.001.patch, HADOOP-13449-HADOOP-13345.002.patch,
> HADOOP-13449-HADOOP-13345.003.patch, HADOOP-13449-HADOOP-13345.004.patch
>
>
> Provide an implementation of the metadata store backed by DynamoDB.
--
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
