[
https://issues.apache.org/jira/browse/HDFS-8058?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621444#comment-14621444
]
Jing Zhao commented on HDFS-8058:
---------------------------------
Thanks for working on this, Zhe. The patch looks good to me overall. Some
comments:
# Any reason to change this function to public?
{code}
- private void setFileReplication(short replication) {
+ public final void setFileReplication(short replication) {
{code}
# The following changes are unnecessary.
{code}
- @Override // INodeFileAttributes
+ @Override
public long getHeaderLong() {
{code}
{code}
- @Override // BlockCollection
+ @Override
public BlockInfo[] getBlocks() {
{code}
{code}
- if (snapshot == CURRENT_STATE_ID || getDiffs() == null) {
+ if(snapshot == CURRENT_STATE_ID || getDiffs() == null)
return getBlocks();
- }
- // find blocks stored in snapshot diffs (for truncate)
FileDiff diff = getDiffs().getDiffById(snapshot);
// note that currently FileDiff can only store contiguous blocks
- BlockInfo[] snapshotBlocks = diff == null ? getBlocks() : diff.getBlocks();
- if (snapshotBlocks != null) {
+ BlockInfo[] snapshotBlocks =
+ diff == null ? getBlocks() : diff.getBlocks();
+ if(snapshotBlocks != null)
return snapshotBlocks;
- }
{code}
{code}
- /** Used during concat to update the BlockCollection for each block */
{code}
{code}
- if (oldBlocks == null) {
+ if (oldBlocks == null)
return 0;
- }
{code}
# The whole FileWithStripedBlocksFeature can be deleted. Maybe We can take one
bit from REPLICATION's 12 bits in the header to represent if the file is with
striped blocks or not. Then need to update INodeFile#isStriped accordingly. The
fsimage related code and protobuf definition also needs to be updated.
# We can call clearBlocks instead of setBlocks(null) in the following code.
{code}
- FileWithStripedBlocksFeature sb = getStripedBlocksFeature();
- if (sb != null) {
- sb.clear();
- }
+ setBlocks(null);
{code}
# We now do not need to call getBlocks().
{code}
public final long computeFileSize(boolean includesLastUcBlock,
- boolean usePreferredBlockSize4LastUcBlock)
{
+ boolean usePreferredBlockSize4LastUcBlock) {
BlockInfo[] blockInfos = getBlocks();
{code}
{code}
- public final QuotaCounts storagespaceConsumedWithStriped() {
+ public final QuotaCounts storagespaceConsumedStriped() {
QuotaCounts counts = new QuotaCounts.Builder().build();
- BlockInfo[] blockInfos = getBlocks();
- if (blockInfos == null || blockInfos.length == 0) {
+ BlockInfo[] blks = getBlocks();
+ if (blks == null || blks.length == 0) {
{code}
> Erasure coding: use BlockInfo[] for both striped and contiguous blocks in
> INodeFile
> -----------------------------------------------------------------------------------
>
> Key: HDFS-8058
> URL: https://issues.apache.org/jira/browse/HDFS-8058
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Affects Versions: HDFS-7285
> Reporter: Yi Liu
> Assignee: Yi Liu
> Attachments: HDFS-8058-HDFS-7285.003.patch,
> HDFS-8058-HDFS-7285.004.patch, HDFS-8058.001.patch, HDFS-8058.002.patch
>
>
> This JIRA is to use {{BlockInfo[] blocks}} for both striped and contiguous
> blocks in INodeFile.
> Currently {{FileWithStripedBlocksFeature}} keeps separate list for striped
> blocks, and the methods there duplicate with those in INodeFile, and current
> code need to judge {{isStriped}} then do different things. Also if file is
> striped, the {{blocks}} in INodeFile occupy a reference memory space.
> These are not necessary, and we can use the same {{blocks}} to make code more
> clear.
> I keep {{FileWithStripedBlocksFeature}} as empty for follow use: I will file
> a new JIRA to move {{dataBlockNum}} and {{parityBlockNum}} from
> *BlockInfoStriped* to INodeFile, since ideally they are the same for all
> striped blocks in a file, and store them in block will waste NN memory.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
