[
https://issues.apache.org/jira/browse/HDFS-1359?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12905322#action_12905322
]
Tsz Wo (Nicholas), SZE commented on HDFS-1359:
----------------------------------------------
The patch looks good. Have some comments below:
- typo
{code}
- * close() also commits the last block of the file by reporting
+ * close() also commits the last block ofExtendedBlockfile by reporting
{code}
- You may have mentioned it somewhere earlier. What is the reason that
ExtendedBlock includes Block as a field but not extending Block?
- In ExtendedBlock.write(..), it should call Block.write(..). Similar to
ExtendedBlock.readFields(..)
- There are a few "TODO remove this", "TODO poolID" and TODO something. When
should they be removed or what todo? It may be better to state it in the
comments. Otherwise, we may forget to do it. It may be useful to add some
special symbols, e.g. EXTENDEDBLOCK_REMOVE_THIS. Then, we can search them
later.
- typo
{code}
+ private void checkPoolId(String poolid) throws IOException {
+ if (!this.poolId.equals(poolId)) {
+ throw new IOException("PoolId " + poolid
+ + " does not belong to expected pool " + poolId);
+ }
+ }
{code}
poolId should be compared with poolid (not itself). In general, I think we
should avoid variable names different only with one character. In this case, I
like to use that_poolId.
- should update protocol version
> HDFS federation: Add BlockPoolID to block
> -----------------------------------------
>
> Key: HDFS-1359
> URL: https://issues.apache.org/jira/browse/HDFS-1359
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: Suresh Srinivas
> Assignee: Suresh Srinivas
> Attachments: HDFS-1359.patch
>
>
> This is a subtask to HDFS-1052 to add BlockPoolID to Block.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
