[
https://issues.apache.org/jira/browse/HDFS-9103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15007307#comment-15007307
]
James Clampffer commented on HDFS-9103:
---------------------------------------
"It can be moved into the bindings/c instead of being a public header."
"The definition needs to be in hdfs.h."
Sounds good to me.
"The comment is redundant. It's also acceptable as we do assume that the
InputStreamImpl class is the only one inheriting the InputStream interface"
I'll strip out the comment.
"It's more clear to call the parameter excluded_nodes."
Sounds good.
"Let's simply inlines the function. Protobuf objects might contain heap objects
so it's important to make the life cycle explicit."
"The unit test can be done through traits."
The reason I factored that into a method was to make it easy to expose to
tests; it's also a fairly self contained block of code so I think it makes
makes sense to separate it regardless. I'd really like to avoid adding traits
just for testing just due to the amount of new/complicated code traits tend to
introduce. Currently all of the pointers used as out parameters point into
InputStream::blocks_ so there shouldn't be any lifecycle issues but I'd be
happy to change them to use value assignment rather than pointer assignment to
get rid of any risk.
"Leaving this part out and have some testings on the bad node tracker itself is
also acceptable."
Does this mean you would be fine if I stripped out the tests for the override
rule, inlined the function again, and added a couple more test cases to
BadDataNodeTracker? I want to make sure I'm getting coverage on the
excluded_nodes parameter.
"Can be simplified with std::find_if()."
Yea good point, will change.
> Retry reads on DN failure
> -------------------------
>
> Key: HDFS-9103
> URL: https://issues.apache.org/jira/browse/HDFS-9103
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
> Reporter: Bob Hansen
> Assignee: James Clampffer
> Fix For: HDFS-8707
>
> Attachments: HDFS-9103.1.patch, HDFS-9103.2.patch,
> HDFS-9103.HDFS-8707.006.patch, HDFS-9103.HDFS-8707.007.patch,
> HDFS-9103.HDFS-8707.008.patch, HDFS-9103.HDFS-8707.009.patch,
> HDFS-9103.HDFS-8707.3.patch, HDFS-9103.HDFS-8707.4.patch,
> HDFS-9103.HDFS-8707.5.patch
>
>
> When AsyncPreadSome fails, add the failed DataNode to the excluded list and
> try again.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
