[
https://issues.apache.org/jira/browse/HADOOP-2656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12590270#action_12590270
]
Konstantin Shvachko commented on HADOOP-2656:
---------------------------------------------
BlockMetadata.
# 3 redundant imports
# HEADER_BUF_SIZE is never read locally
# upgradeToCurVersion() should be a part of GenerationStampUpgrade
also there is problem with indentations.
# It is better to place all upgrade-related code into GenerationStampUpgrade.
Say, Header.needsUpgrade() should be static needsUpgrade(Header).
# not sure all 3 variants of readHeader() are needed. At least one is not
called anywhere.
# I propose to restructure this class in the following way:
make BlockMetadata.Header class the top level class rather than the inner,
rename it to BlockMetadataHeader.
In other words this means getting rid of the inner class Header, moving all
its contents up to BlockMetadata,
and then renaming the latter.
# line 160: should not use deprecated getMetaFile() method. You will need old
meta data file names
as long as the upgrade exists, so it is better to have a
getPreGenerationMetaFile() or smth. in the
corresponding upgrade object.
Datanode
# New comment about the return value in startDistributedUpgradeIfNeedes() is
wrong, pls remove.
# Starting block scanner in offerService() you should check that the scaner has
already been started,
otherwise you will be starting it every hour after the upgrade is finished.
DatanodeBlockInfo
- In detachBlock() can you use static getMetaFile(File, Block) rather than
passing FSDataset into it.
DataBlockScanner
- assignInitialVerificationTimes() should use tmpBlock.set() rather then
assigning fields directly.
FSDataset
- Deprecated getMetaFile() the one that is "used only by Upgrade code" is not
public and could be
moved directly to the upgrade code rather than deprecated.
FSEditLog
# In loadFSEdit() line "if (logVersion < -13)" should read as
{code}
if (logVersion <= -13)
{code}
# Introduction of BlockTwo for reading old versions of block layout is kinda
confusing.
I'd rather explicitly read the array length and then each of the two fields in
a loop,
which you have anyway since the prehistoric generation stamp needs to be
assigned.
FSImage
- line "if (-14 < imgVersion)" should read
{code}
if (imgVersion <= -13)
{code}
swap the if and else bodies.
FSNamesystem
NameNode
NamenodeFsck
UnderReplicatedBlocks
- This is all mostly about replacing block.getBlockName() with block.toString().
UpgradeManager
{code}
boolean isUpgradeCompleted() {
return currentUpgrade == null;
}
{code}
GenerationStampUpgrade
# same as in BlockMetadata(7) in line 260.
# ";;" remove one
# There is a lot of common code in BlockCrcUpgrade and GenerationStampUpgrade.
Many classes and types are literally (past©) the same:
#- UpgradeStatus (with the STARTED field never used)
#- DNInfo
#- BlockLevelStats
#- GenerationStampUpgradeStatusReport = BlockCrcUpgradeStatusReport
#- DatanodeStatsCommand
#- I probably missed some.
# We should make them all static classes and move either to the base classes
if the actions are common and useful for other generations of upgrades or
create
UpgradeUtils class and keep them there.
# The important question is how do we test the generation stamp upgrade?
With crcs we had a test plan and were testing different scenarios of failure of
the upgrade at different stages.
We should either do the same for genStamps or have a unit test, that tests the
scenarios automatically.
General
# Please look for empty line changes in the final patch.
# Please replace /\* \*/ comments with /\*\* \*/ for classes, class methods,
members, and constants.
> Support for upgrading existing cluster to facilitate appends to HDFS files
> --------------------------------------------------------------------------
>
> Key: HADOOP-2656
> URL: https://issues.apache.org/jira/browse/HADOOP-2656
> Project: Hadoop Core
> Issue Type: Bug
> Components: dfs
> Reporter: dhruba borthakur
> Assignee: dhruba borthakur
> Fix For: 0.18.0
>
> Attachments: upgradeGenStamp4.patch, upgradeGenStamp5.patch
>
>
> HADOOP-1700 describes the design for supporting appends to HDFS files. This
> design requires a distributed-upgrade to existing cluster installations. The
> design specifies that the DataNode persist the 8-byte BlockGenerationStamp in
> the block metadata file. The upgrade code will introduce this new field in
> the block metadata file and initialize this value to 0.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
