[
https://issues.apache.org/jira/browse/HDFS-7866?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15074997#comment-15074997
]
Kai Zheng commented on HDFS-7866:
---------------------------------
Some comments so far. Will continue the review tomorrow.
1. Ref. below codes:
{code}
+ public ErasureCodingPolicy(ECSchema schema, int cellSize) {
+ this.name = composePolicyName(schema, cellSize);
+ this.schema = schema;
+ this.cellSize = cellSize;
+ id = getNextID();
+ }
{code}
How about this to reuse the constructor.
{code}
public ErasureCodingPolicy(ECSchema schema, int cellSize) {
this(composePolicyName(schema, cellSize), cellSize);
}
{code}
2. See the codes. What will happen when NN restarts?
{code}
+ private static final AtomicInteger ID_GEN = new AtomicInteger(0);
+
{code}
{code}
+ private static short getNextID() {
+ return (short) ID_GEN.getAndIncrement();
+ }
{code}
3. In below codes, not sure better or not use 0 or 1 for the block_replication
value in striping case. Would you check related codes?
{code}
- this.block_replication = (short)block_replication;
+ this.block_replication = ecPolicy == null ? (short) block_replication : 0;
{code}
4. A little confused.
{code}
- // can only afford one replica loss
+ // there is less than a third as many blocks as requested;
{code}
5. I guess better to use only one map for the most often case to avoid out of
sync in two maps. For lookup using the other key, simple iteration may do the
work.
{code}
- this.activePolicies = new TreeMap<>();
- for (ErasureCodingPolicy policy : SYS_POLICY) {
- activePolicies.put(policy.getName(), policy);
+ this.activePoliciesByName = new TreeMap<>();
+ this.activePoliciesByID = new TreeMap<>();
+ for (ErasureCodingPolicy policy : SYS_POLICIES) {
+ activePoliciesByName.put(policy.getName(), policy);
+ activePoliciesByID.put(policy.getId(), policy);
}
{code}
> Erasure coding: NameNode manages EC schemas
> -------------------------------------------
>
> Key: HDFS-7866
> URL: https://issues.apache.org/jira/browse/HDFS-7866
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: Kai Zheng
> Assignee: Rui Li
> Attachments: HDFS-7866-v1.patch, HDFS-7866-v2.patch,
> HDFS-7866-v3.patch, HDFS-7866.4.patch, HDFS-7866.5.patch, HDFS-7866.6.patch,
> HDFS-7866.7.patch
>
>
> This is to extend NameNode to load, list and sync predefine EC schemas in
> authorized and controlled approach. The provided facilities will be used to
> implement DFSAdmin commands so admin can list available EC schemas, then
> could choose some of them for target EC zones.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)