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

Andrew Wang commented on HDFS-12222:
------------------------------------

Hi Huafeng, thanks for working on this, I took a look at the 005 patch, a few 
review comments:

* I recommend moving the BlockLocation[] format example to 
FileSystem#getFileBlockLocations and FileContext#getFileBlockLocations, which 
are public APIs. Then, we can link to it from the other code locations.
* Related, the javadoc links to TestDistributedFileSystemWithECFile don't work 
since prod code doesn't depend on test, recommend instead doing the above 
change and linking to the public method
* Recommend adding a link to FileSystem#getFileBlockLocations to 
LocatedFileStatus#getBlockLocations.
* Recommend adding a format explanation for a single BlockLocation (replicated 
and EC) to BlockLocation as well, along with the link to FileSystem and 
FileContext.

Regarding this blurb:

{code}
   * In HDFS implementation, the BlockLocation of returned LocatedFileStatus
   * will have different formats for replicated and erasure coded file. Please
   * refer to HDFS for more details.
{code}

I'd prefer we drop this in most places, since this format should be compatible 
for downstreams, and it'll be well documented on the public API classes noted 
above.

> Add EC information to BlockLocation
> -----------------------------------
>
>                 Key: HDFS-12222
>                 URL: https://issues.apache.org/jira/browse/HDFS-12222
>             Project: Hadoop HDFS
>          Issue Type: Bug
>    Affects Versions: 3.0.0-alpha1
>            Reporter: Andrew Wang
>            Assignee: Huafeng Wang
>              Labels: hdfs-ec-3.0-nice-to-have
>         Attachments: HDFS-12222.001.patch, HDFS-12222.002.patch, 
> HDFS-12222.003.patch, HDFS-12222.004.patch, HDFS-12222.005.patch
>
>
> HDFS applications query block location information to compute splits. One 
> example of this is FileInputFormat:
> https://github.com/apache/hadoop/blob/d4015f8628dd973c7433639451a9acc3e741d2a2/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/FileInputFormat.java#L346
> You see bits of code like this that calculate offsets as follows:
> {noformat}
>     long bytesInThisBlock = blkLocations[startIndex].getOffset() + 
>                           blkLocations[startIndex].getLength() - offset;
> {noformat}
> EC confuses this since the block locations include parity block locations as 
> well, which are not part of the logical file length. This messes up the 
> offset calculation and thus topology/caching information too.
> Applications can figure out what's a parity block by reading the EC policy 
> and then parsing the schema, but it'd be a lot better if we exposed this more 
> generically in BlockLocation instead.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to