[
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)
