hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rakesh R (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-8450) Erasure Coding: Consolidate erasure coding zone related implementation into a single class
Date Thu, 04 Jun 2015 13:12:38 GMT

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

Rakesh R commented on HDFS-8450:
--------------------------------

Thanks again [~drankye] for the reviews.

bq. 1. Minor, please update the header comment "Helper class to perform erasure coding zone
related operations."
OK. I will modify.
bq. 2. In createErasureCodingZone, checkSuperuserPrivilege, checkOperation and writeLock were
called two times. I'm not very sure about this, maybe Vinayakumar B could give a comment?
+checkSuperuserPrivilege+
I referred other FSNamesystem implementations except encryption related implementaion all
other cases {{checkSuperuserPrivilege}} function is called only once. It looks like not consistently
followed. It would be good to find best practise and follow the same. Any suggestions?

+checkOperation+
All the other FSN implementations are following the same pattern, so I feel two times call
is OK.

+writeLock+
First {{writeLock}} is by FSNamesystem and second is by FsDirectory. All others are following
the same pattern, so I feel two times call is OK.

[~vinayrpet], could you please correct me if am missing anything. Thanks!

bq. 3. getErasureCodingZoneForPath could be private as internally used? Note no lock here
so it shouldn't be used by outside.
Yes, Agreed.
bq. 4. In isInErasureCodingZone, no lock and operation check as did in createErasureCodingZone.

I think there is one improvement I have to do is to move the {{fsn.writeLock()}} outside the
helper class. After referring other {{FSDir*Op}} implementations, it seems that the caller
should acquire the {{fsn.writeLock/fsn.readLock}} and then inside the helper class it is taking
{{fsd.writeLock()/fsd.readLock()}} depends on the operation. I will refactor {{#createErasureCodingZone}}
and also will acquire necessary lock for {{isInErasureCodingZone}} function.

bq.  5. Maybe we could have a private function to transform src to iip?
OK, I will do it.


> Erasure Coding: Consolidate erasure coding zone related implementation into a single
class
> ------------------------------------------------------------------------------------------
>
>                 Key: HDFS-8450
>                 URL: https://issues.apache.org/jira/browse/HDFS-8450
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Rakesh R
>            Assignee: Rakesh R
>         Attachments: HDFS-8450-FYI.patch, HDFS-8450-HDFS-7285-00.patch, HDFS-8450-HDFS-7285-01.patch,
HDFS-8450-HDFS-7285-02.patch, HDFS-8450-HDFS-7285-03.patch, HDFS-8450-HDFS-7285-04.patch,
HDFS-8450-HDFS-7285-05.patch, HDFS-8450-HDFS-7285-07.patch
>
>
> The idea is to follow the same pattern suggested by HDFS-7416. It is good  to consolidate
all the erasure coding zone related implementations of {{FSNamesystem}}. Here, proposing {{FSDirErasureCodingZoneOp}}
class to have functions to perform related erasure coding zone operations.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message