hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Manoj Govindassamy (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (HDFS-11382) Persist Erasure Coding Policy ID in a new optional field in INodeFile in FSImage
Date Thu, 23 Feb 2017 05:06:44 GMT

     [ https://issues.apache.org/jira/browse/HDFS-11382?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel

Manoj Govindassamy updated HDFS-11382:
    Attachment: HDFS-11382.02.patch

Thanks for the review [~andrew.wang].  Attached v02 patch which addresses the following.

bq. I'm wondering if we should make isStriped just be return getBlockType() == STRIPED, looks
equivalent to me.

bq. FSImageFormatPBINode, it'd be better to check if the policy is present with f.hasErasureCodingPolicyID..
Makes sense. Fixed the same way.

bq. This relates to the comment in toLong about this being hacky, I dug up this comment from
Zhe during the initial review..Seems like this JIRA is a good time to fix the hack
Sure. Fixed the constructor in {{INodeFile}} to have distinct params for replication and ecPolicyID,
fixed all the callers. Made necessary checks for the validity of the argument passed in based
on the BlockType context.

bq. FSImageFormatPBInode#buildINodeFile, it looks like we unconditionally set the EC policy
Fixed this. Now, ec policy id is set only if the file is striped.

bq. INodeFile#getFileReplication, IIRC S3 returns a replication factor of 1.
Check S3 filesystem and it does return 1 for replication. Followed the same model and made
{{INodeFile}} getReplication*() to return 1.

bq. Is the PBXMLWriter change intended for this JIRA?
I added few EC files in the setup in {{TestOfflineImageViewer}} so that i can test this jira
fix of persisted ec policy id. After adding these EC files to FSImage, one of the XML tests
in TestOfflineImageViewer started failing, complaining about the unexpected children section
for the element <blocktype>. Followed similar handling done for storage policy in dumpingINode
for blockType and it solved the problem. So, my changes exposed a new problem around XML processor
in FSImage and its fixed. 

> Persist Erasure Coding Policy ID in a new optional field in INodeFile in FSImage
> --------------------------------------------------------------------------------
>                 Key: HDFS-11382
>                 URL: https://issues.apache.org/jira/browse/HDFS-11382
>             Project: Hadoop HDFS
>          Issue Type: Task
>          Components: hdfs
>    Affects Versions: 3.0.0-alpha1
>            Reporter: Manoj Govindassamy
>            Assignee: Manoj Govindassamy
>         Attachments: HDFS-11382.01.patch, HDFS-11382.02.patch
> For Erasure Coded files, replication field in INodeFile message is re-used  to store
the EC Policy ID. 
> *FSDirWriteFileOp#addFile*
> {noformat}
>   private static INodesInPath addFile(
>       FSDirectory fsd, INodesInPath existing, byte[] localName,
>       PermissionStatus permissions, short replication, long preferredBlockSize,
>       String clientName, String clientMachine)
>       throws IOException {
>     .. .. ..
>     try {
>       ErasureCodingPolicy ecPolicy = FSDirErasureCodingOp.
>           getErasureCodingPolicy(fsd.getFSNamesystem(), existing);
>       if (ecPolicy != null) {
>         replication = ecPolicy.getId();   <===
>       }
>       final BlockType blockType = ecPolicy != null?
>           BlockType.STRIPED : BlockType.CONTIGUOUS;
>       INodeFile newNode = newINodeFile(fsd.allocateNewInodeId(), permissions,
>           modTime, modTime, replication, preferredBlockSize, blockType);
>       newNode.setLocalName(localName);
>       newNode.toUnderConstruction(clientName, clientMachine);
>       newiip = fsd.addINode(existing, newNode, permissions.getPermission());
> {noformat}
> With HDFS-11268 fix, {{FSImageFormatPBINode#Loader#loadInodeFile}} is rightly getting
the EC ID from the replication field and then uses the right Policy to construct the blocks.
> *FSImageFormatPBINode#Loader#loadInodeFile*
> {noformat}
>       ErasureCodingPolicy ecPolicy = (blockType == BlockType.STRIPED) ?
>           ErasureCodingPolicyManager.getPolicyByPolicyID((byte) replication) :
>           null;
> {noformat}
> The original intention was to re-use the replication field so the in-memory representation
would be compact. But, this isn't necessary for the on-disk representation. replication is
an optional field, and if we add another optional field for the EC policy, it won't be any
extra space.
> Also, we need to make sure to have the appropriate asserts in place to make sure both
fields aren't set for the same INodeField.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: hdfs-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-help@hadoop.apache.org

View raw message