[
https://issues.apache.org/jira/browse/HADOOP-2345?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12567233#action_12567233
]
shv edited comment on HADOOP-2345 at 2/8/08 2:49 PM:
---------------------------------------------------------------------
# ClientProtocol.versionID should be incremented.
# NameNode.fsync() should use @inheritDoc.
# FSDirectory.delete(String src, INode[] old) needs documentation on the
parameter "old".
# Since you return only one INode I'd change prototype of unprotectedDelete
{code}
Block[] unprotectedDelete(String src, long modificationTime, INode[] old)
{code}
to
{code}
INode unprotectedDelete(String src, long modificationTime, Collection<Block>
deletedBlocks)
{code}
Replacing Block[] to Collection<Block> will probably be required, since we do
not know in
advance how many blocks will be returned. It's a good thing anyway, since
collections
are more flexible than arrays.
# getFileINode() I am not sure about the consequences of removing
waitForReady() from it.
I understand you need to look for files under construction before the entire
image is loaded,
but doesn't it break behavior of other methods. E.g. one will be able to call
getBlockLocations() or startFile() before the image is loaded.
# OP_ADD sounds good to me. It can mean adding a file or adding a block.
{code}
OP_OPEN = 9; // create for write
{code}
sounds controversial, doesn't it?
So may be you should retain OP_ADD in favor replacing it with OP_OPEN.
# OP_GENSTAMP: I'd propose OP_SET_GENERATION instead.
# You use different comparisons, which are equivalent but confusing in
detecting the version they refer to.
{code}
logVersion <= -12
logVersion < -11
{code}
I'd just use logVersion <= -12 everywhere.
# Why generation stamp needs to be serialized as a one element array?
Arrays are appropriate if several members need to be serialized at once.
In the case you can simply in.readLong() / writeLong().
The same as in FSImage.
# Do we want to introduce something like
{code}
ClientID {
protected StringBytesWritable clientName; // lease holder
protected StringBytesWritable clientMachine;
}
{code}
Is there a use case for this?
# FSImage.loadFSImage(File)
You want to "read in the last generation stamp" after imgVersion and numFile
are read and sorted out.
Somewhere after
{code}
this.layoutVersion = imgVersion;
{code}
Otherwise loading of pre-version images will fail. It's unlikely somebody will
need that, but still.
# FSNamesystem.generationStamp: Could you please use JavaDoc-style comments for
the description of this member.
# FSDirectory.closeLease() is confusing because it has nothing to do with
leases.
I was first wondering why it is a part of FSDirectory while other lease methods
are in FSNamesystem,
but then realized that it is just closing the file. Please rename, and also the
comments to the calls
of closeLease() say "persist block allocations for this file" which does not
reflect the actual action
any more.
# GenerationStamp may implement WritableComparable rather than Writable,
Comparable
# I like that serialization code for INodeUnderConstruction is separated from
the loadFSImage mainstream
code. I am not so sure it should implement Writable because this class is not
supposed to be a part of
any communications. Alternatively we could define methods
{code}
FSImage.readINodeUnderConstruction(in, INodeUnderConstruction)
FSImage.writeINodeUnderConstruction(out, INodeUnderConstruction)
{code}
or
{code}
static INodeUnderConstruction.readINodein, INodeUnderConstruction)
static INodeUnderConstruction.writeINodeout, INodeUnderConstruction)
{code}
was (Author: shv):
- ClientProtocol.versionID should be incremented.
- NameNode.fsync() should use @inheritDoc.
- FSDirectory.delete(String src, INode[] old) needs documentation on the
parameter "old".
- Since you return only one INode I'd change prototype of unprotectedDelete
{code}
Block[] unprotectedDelete(String src, long modificationTime, INode[] old)
{code}
to
{code}
INode unprotectedDelete(String src, long modificationTime, Collection<Block>
deletedBlocks)
{code}
Replacing Block[] to Collection<Block> will probably be required, since we do
not know in
advance how many blocks will be returned. It's a good thing anyway, since
collections
are more flexible than arrays.
- getFileINode() I am not sure about the consequences of removing
waitForReady() from it.
I understand you need to look for files under construction before the entire
image is loaded,
but doesn't it break behavior of other methods. E.g. one will be able to call
getBlockLocations() or startFile() before the image is loaded.
- OP_ADD sounds good to me. It can mean adding a file or adding a block.
{code}
OP_OPEN = 9; // create for write
{code}
sounds controversial, doesn't it?
So may be you should retain OP_ADD in favor replacing it with OP_OPEN.
- OP_GENSTAMP: I'd propose OP_SET_GENERATION instead.
- You use different comparisons, which are equivalent but confusing in
detecting the version they refer to.
{code}
logVersion <= -12
logVersion < -11
{code}
I'd just use logVersion <= -12 everywhere.
- Why generation stamp needs to be serialized as a one element array?
Arrays are appropriate if several members need to be serialized at once.
In the case you can simply in.readLong() / writeLong().
The same as in FSImage.
- Do we want to introduce something like
{code}
ClientID {
protected StringBytesWritable clientName; // lease holder
protected StringBytesWritable clientMachine;
}
{code}
Is there a use case for this?
- FSImage.loadFSImage(File)
You want to "read in the last generation stamp" after imgVersion and numFile
are read and sorted out.
Somewhere after
{code}
this.layoutVersion = imgVersion;
{code}
Otherwise loading of pre-version images will fail. It's unlikely somebody will
need that, but still.
- FSNamesystem.generationStamp Could you please use JavaDoc-style comments for
the description of this member.
- FSDirectory.closeLease() is confusing because it has nothing to do with
leases.
I was first wondering why it is a part of FSDirectory while other lease methods
are in FSNamesystem,
but then realized that it is just closing the file. Please rename, and also the
comments to the calls
of closeLease() say "persist block allocations for this file" which does not
reflect the actual action
any more.
- GenerationStamp may implement WritableComparable rather than Writable,
Comparable
- I like that serialization code for INodeUnderConstruction is separated from
the loadFSImage mainstream
code. I am not so sure it should implement Writable because this class is not
supposed to be a part of
any communications. Alternatively we could define methods
{code}
FSImage.readINodeUnderConstruction(in, INodeUnderConstruction)
FSImage.writeINodeUnderConstruction(out, INodeUnderConstruction)
{code}
or
{code}
static INodeUnderConstruction.readINodein, INodeUnderConstruction)
static INodeUnderConstruction.writeINodeout, INodeUnderConstruction)
{code}
> new transactions to support HDFS Appends
> ----------------------------------------
>
> Key: HADOOP-2345
> URL: https://issues.apache.org/jira/browse/HADOOP-2345
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: dhruba borthakur
> Assignee: dhruba borthakur
> Fix For: 0.17.0
>
> Attachments: AppendTransactions.patch, AppendTransactions2.patch
>
>
> This JIRA adresses the changes needed to the transaction mechanism to support
> appending data to existing HDFS files. The details of this design is
> documented in HADOOP-1700.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
