[
https://issues.apache.org/jira/browse/HDFS-517?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12745712#action_12745712
]
Konstantin Shvachko commented on HDFS-517:
------------------------------------------
> Two general comments:
As we talked about it, this logic belongs to changes related to block reporting
and blockReceived logic. Not to this patch.
> HdfsConstants.java
1. replaced with "created for replication and relocation only".
2. I disagree. BlockUCState is a block under construction state. Finalized
block has other states like under-replicated, over-replicated, corrupt, etc.
The states this enum defines referred to an under construction block only.
Calling it a state would be too generic. I actually did this in the beginning,
but realized later it is not correct.
> BlockInfo.java
1. Constructors need to be protected, because we should always set the most
strict restrictions possible. When a need arise we may relax them. There is no
such need yet.
2. Yes NullPointerException will be a bug in our code.
3. I changed member blockState to blockUCState. You are right getters and
setters should match the member name. See my comment about state above.
4. This is checked in convertLastBlockToUnderConstruction(). I really want to
avoid double checking I'll add an assert here.
Current append logic does not initialize targets in file inode. We will need to
introduce it later.
> BlockUnderConstruction.java
1. Added an assert.
2. Current code does not allow that. Lets loosen restrictions when needed
rather than in advance.
3. Yes we should be prepared for repetitive calls if client gets disconnected.
4. No it starts from i = 1.
5. There is Block.set(id, len, gs). Set for state will be added when we will be
changing uc states.
> BlockManager.java
1. The JavaDoc explains it.
2. completeBlock() needs to know which block to complete. It does not make
sense to search for it.
> BlocksMap.java
This is a correct observation.
> FSdirectory.java
No, because this is how addToParent is currently used. What makes sense is to
merge addToParent and unprotectedAddFile(). They are the same. I leave it for
after-append re-factoring.
> FSNamesystem
Changed error message.
> INodeFile.java
The advantage of returning a generic type is that you do not need to convert
types if you know you need for example BlockInfoUnderConstruction. You just
create a variable of that type and you are done. Otherwise you will need to
perform an explicit type conversion.
> Introduce BlockInfoUnderConstruction to reflect block replica states while
> writing.
> -----------------------------------------------------------------------------------
>
> Key: HDFS-517
> URL: https://issues.apache.org/jira/browse/HDFS-517
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: name-node
> Affects Versions: 0.21.0
> Reporter: Konstantin Shvachko
> Assignee: Konstantin Shvachko
> Fix For: Append Branch
>
> Attachments: Append-UCBlock-h265.patch, Append-UCBlock.patch,
> Append-UCBlock.patch, Append-UCBlock.patch
>
>
> Currently when a block is created its locations are stored in
> {{INodeFileUnderConstruction.targets}}, which correspond to the last
> allocated block. With the new append design we will need to keep track of
> block replicas for several blocks rather than just the last one.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
