[
https://issues.apache.org/jira/browse/HADOOP-13448?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15468963#comment-15468963
]
Chris Nauroth commented on HADOOP-13448:
----------------------------------------
[~fabbri], thanks for continuing work on this while I was out. The changes
look good to me. Here are a few comments. Are you interested in doing another
revision, or should I?
bq. For PathMetadata#toString(), it'd be nice to have isDirectory flag in the
output.
This is an old comment referring to revision 001, but I figured I'd respond
anyway. The {{toString}} output did indicate whether it was referring to a
directory or a file due to inclusion of the specific subclass name:
{{DirectoryPathMetadata}} or {{FilePathMetadata}}.
bq. Could also use a private boolean isDirectory instead introducing separate
private class to return true/false.
The private class separation I was trying earlier was an attempt to set us up
to reduce memory footprint for metadata instances by only storing certain
members in relevant subclasses that need them. This may be premature of me, so
please feel free to put a boolean member variable in there instead.
bq. I use a type bound to allow them to hold any subclass of FileStatus, e.g.
S3AFileStatus.
This is nice, but I expect in practice we'll instantiate through
{{ReflectionUtils#newInstance}} and lose the type bounds unless we do an unsafe
cast. (Thanks, Java!)
bq. Should MetadataStore be an interface?
I had set it up as an abstract base class just for convenience of
implementation inheritance getting a {{Logger}} instance. At this point,
interface does look like the more appropriate choice.
bq. How do folks feel about this API consuming/producing FileStatus'?
I think this is fine.
The presence of the {{DirListingMetadata#put}} method implies mutable usage.
Should this class provide thread safety, or is there an expectation that the
caller will enforce thread-safe access? Either way, it would be good to add a
comment about thread safety to the class-level JavaDocs.
Does {{DirListingMetadata}} need a method to return all children in the
listing? In your prototype on HADOOP-13345, there was
{{CachedDirectory#getFileStatuses}}.
Similarly, does {{DirListingMetadata}} need mutator methods for
{{isAuthoritative}}?
I suppose a higher-level decision for all of the above is mutable vs. immutable
objects. For a {{MetadataStore}} implemented by a persistent back-end, the
objects probably need to be reconsituted after each query, so immutable would
work well. OTOH, for the in-memory implementation, it's possibly more
efficient to allow mutability.
Jenkins seems to be rebooting at the moment, so I couldn't review the
Checkstyle and JavaDoc warnings.
> S3Guard: Define MetadataStore interface.
> ----------------------------------------
>
> Key: HADOOP-13448
> URL: https://issues.apache.org/jira/browse/HADOOP-13448
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Reporter: Chris Nauroth
> Assignee: Chris Nauroth
> Attachments: HADOOP-13448-HADOOP-13345.001.patch,
> HADOOP-13448-HADOOP-13345.002.patch
>
>
> Define the common interface for metadata store operations. This is the
> interface that any metadata back-end must implement in order to integrate
> with S3Guard.
--
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
