hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "James Clampffer (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-9144) Refactor libhdfs into stateful/ephemeral objects
Date Thu, 19 Nov 2015 22:34:11 GMT

    [ https://issues.apache.org/jira/browse/HDFS-9144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15014569#comment-15014569

James Clampffer commented on HDFS-9144:

Some feedback, as well as some questions to make sure I understand everything correctly:

-needs to be rebased to head now that dn retry is added
-The synchronous FileSystem::Open isn't declared pure virtual, it probably should be.
-hdfsConnect looks like it can leak an IoService if FileSystem::New returns null.  Unlikely
but possible as far as I can tell.
-Why is GetRandomClientName declared inline? Is this just to trick the compiler into letting
it stay in the header?  Probably not a whole lot to gain by inlining a non-leaf function that
large; consider moving implementation to a .cc file?
-DataNodeConnectionImpl could probably have it's constructor implementation in a .cc file.
 I think you could set conn_ in the initializer list to get rid of the conn_.reset call in
the ctor.
-BlockReaderImpl has a shared_ptr<DataNodeConnection> member, what is this being shared
with?  Is that just so it can pass a reference to itself to continuations?
-DataTransferSaslStream::Handshake still uses a templated callback which I think a lot of
this patch is moving away from.  Fixing up those little pieces can be deferred to a later
jira as it looks like nothing is calling it at the moment.
-Have you been able to run anything against the C++ or C APIs, with and without memcheck,
to verify that the refactor didn't introduce any subtle bugs?

Overall I think it's a solid set of abstractions.

> Refactor libhdfs into stateful/ephemeral objects
> ------------------------------------------------
>                 Key: HDFS-9144
>                 URL: https://issues.apache.org/jira/browse/HDFS-9144
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: hdfs-client
>    Affects Versions: HDFS-8707
>            Reporter: Bob Hansen
>            Assignee: Bob Hansen
>         Attachments: HDFS-9144.HDFS-8707.001.patch, HDFS-9144.HDFS-8707.002.patch, HDFS-9144.HDFS-8707.003.patch
> In discussion for other efforts, we decided that we should separate several concerns:
> * A posix-like FileSystem/FileHandle object (stream-based, positional reads)
> * An ephemeral ReadOperation object that holds the state for reads-in-progress, which
> * An immutable FileInfo object which holds the block map and file size (and other metadata
about the file that we assume will not change over the life of the file)

This message was sent by Atlassian JIRA

View raw message