[
https://issues.apache.org/jira/browse/HDFS-7056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14254369#comment-14254369
]
Konstantin Shvachko commented on HDFS-7056:
-------------------------------------------
??calling the first argument "target" rather than "src"? "src" and "dst" make
sense for rename??
This is traditional. If you check {{ClientProtocol}} you should see all methods
there use {{src}} to identify the file name parameter. Including {{create}},
{{append}}, {{complete}}, {{setReplication}}, etc. One can interpret it as a
*source path for the operation*, rather than as an opposite to *destination*.
Do you really want to make an exclusion for {{truncate}} and call it {{target}}?
??I would still like to see these functions return an enum ... people will
assume that if it returns false, the operation failed.??
I see some issues with that:
- In java failure of an operation is signalled by throwing an exception. This
is how most of our APIs do and should work.
- {{delete()}} according to the documentation "returns true only if the
existing file or directory was actually removed".
- mkdirs() in fact always returns true as per NameNode implementation. Could
have been of void type.
- {{boolean truncate()}} is modelled after {{boolean recoverLease()}}. Both
return true if the file is known to be closed upon return from the method.
If you feel strong about returning enum, which I am not, we can create a new
jira to change both return values. It will break wire compatibility for
recoverLease() though.
??calling this bytesToRemove rather than lastBlockDelta???
Sure.
??{{recoveryBlock}} seems like a very generic name for this.??
Makes sense. How about {{truncateBlock}}?
??There isn't any default given here for newBlockId, so what's going to happen
in a mixed-version scenario???
Very good point! Thanks. We should add an explicit default value and treat it
under the assumption that old DNs cannot do truncate recovery.
??Can we have an accessor for this in ReplicaUnderRecovery? I see this pattern
in a lot of places.??
I found only one occurence of this - the one you identified. There are other
places that compare block Ids, but they are applied to Blocks rather than
ReplicaUnderRecovery.
??This code writes out all of the fields in truncateBlock: blockId, numBytes,
generationStamp. But we only care about blockId and generationStamp, right???
In fact, all three fields are needed. We need to know newLength when we restart
truncate recovery after reading edits.
Basically, with copy-on-truncate we need two blocks the old one with the old
length and the new with the new length. Before snapshots we needed only the new
length and the gen stamp, because there was no replica copy. We had only the
newLength as a field back then. truncateBlock was added to support snapshots.
??Missing braces around "if".??
Will fix.
??INodeFile: can these changes be split out into a separate patch???
What do you mean?
INodeFile, AbstractINodeDiffList, and the other snapshot stuff are an intrinsic
part of this implementation of the snapshot support for truncate. This is the
separate jira. And that is why we keep it in two separate patches: one in
HDFS-3071 and another one here.
We will incorporate your suggestions shortly.
> Snapshot support for truncate
> -----------------------------
>
> Key: HDFS-7056
> URL: https://issues.apache.org/jira/browse/HDFS-7056
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: namenode
> Affects Versions: 3.0.0
> Reporter: Konstantin Shvachko
> Assignee: Plamen Jeliazkov
> Attachments: HDFS-3107-HDFS-7056-combined.patch,
> HDFS-3107-HDFS-7056-combined.patch, HDFS-3107-HDFS-7056-combined.patch,
> HDFS-3107-HDFS-7056-combined.patch, HDFS-3107-HDFS-7056-combined.patch,
> HDFS-3107-HDFS-7056-combined.patch, HDFS-3107-HDFS-7056-combined.patch,
> HDFS-3107-HDFS-7056-combined.patch, HDFS-3107-HDFS-7056-combined.patch,
> HDFS-7056.patch, HDFS-7056.patch, HDFS-7056.patch, HDFS-7056.patch,
> HDFS-7056.patch, HDFS-7056.patch, HDFS-7056.patch, HDFS-7056.patch,
> HDFSSnapshotWithTruncateDesign.docx
>
>
> Implementation of truncate in HDFS-3107 does not allow truncating files which
> are in a snapshot. It is desirable to be able to truncate and still keep the
> old file state of the file in the snapshot.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
