[
https://issues.apache.org/jira/browse/HDFS-2602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13167911#comment-13167911
]
Todd Lipcon commented on HDFS-2602:
-----------------------------------
{code}
+ public static boolean sharedEditsDir(Configuration conf) {
+ return null != conf.get(DFS_NAMENODE_SHARED_EDITS_DIR_KEY);
{code}
I think this would be better named {{usesSharedEditsDir}} or
{{isSharedEditsDirConfigured}}
----
{code}
for(Iterator<DatanodeDescriptor> it = blocksMap.nodeIterator(blk);
it.hasNext();) {
final DatanodeDescriptor d = it.next();
+ assert d != null;
{code}
I don't think this new assert is very useful. But let's keep the new one just
below it.
----
In {{getInodeFile}}:
- better to capitalize this as{{getINodeFile}}, I think
- no need for the local variable {{file}} here
----
In {{updateBlocks}}:
{code}
+ INodeFile file = (INodeFile)getInodeFile(fsDir, addCloseOp.path);
{code}
Unnecesary cast
{code}
+ if (oldBlock instanceof BlockInfoUnderConstruction) {
+ fsNamesys.getBlockManager().forceCompleteBlock(
+ (INodeFileUnderConstruction)file,
+ (BlockInfoUnderConstruction)oldBlock);
+ }
{code}
What's the logic behind this? I think there are some cases in which there would
be multiple OP_ADDs for the same file with the same blocks. For example, the
client side {{hflush()}} call can trigger {{persistBlocks}}, but in the case of
HA, the last block would already have been persisted by {{getAdditionalBlock}}
as well. So the last block would be prematurely marked as COMPLETE in this case.
{code}
+ } else if (addCloseOp.blocks.length > oldBlocks.length) {
+ // We're adding a block
+ assert addCloseOp.blocks.length - 1 == oldBlocks.length;
{code}
Not sure this is quite right either - in the non-HA case, we could have the
following sequence of events:
- Client opens file (causes OP_ADD)
- Client writes one block
- Client writes half of second block
- Client calls hflush() which triggers {{persistBlocks}}
This will cause an OP_ADD that adds two blocks rather than just one.
----
- Still has the new {{triggerDeletionReport}} code in {{DatanodeAdapter}}
- Can you locally change the default for {{persistBlocks}} to true and run all
of the unit tests? In particular want to make sure that stuff like lease
recovery continues to work properly.
- Did you run all the unit tests locally?
> Standby needs to maintain BlockInfo while following edits
> ---------------------------------------------------------
>
> Key: HDFS-2602
> URL: https://issues.apache.org/jira/browse/HDFS-2602
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ha
> Affects Versions: HA branch (HDFS-1623)
> Reporter: Todd Lipcon
> Assignee: Aaron T. Myers
> Priority: Critical
> Attachments: HDFS-2602.patch, HDFS-2602.patch
>
>
> As described in HDFS-1975:
> When we close a file, or add another block to a file, we write OP_CLOSE or
> OP_ADD in the txn log. FSEditLogLoader, when it sees these types of
> transactions, creates new BlockInfo objects for all of the blocks listed in
> the transaction. These new BlockInfos have no block locations associated. So,
> when we close a file, the SBNN loses its block locations info for that file
> and is no longer "hot".
> I have an ugly hack which copies over the old BlockInfos from the existing
> INode, but I'm not convinced it's the right way. It might be cleaner to add
> new opcode types like OP_ADD_ADDITIONAL_BLOCK, and actually treat OP_CLOSE as
> just a finalization of INodeFileUnderConstruction to INodeFile, rather than
> replacing block info at all.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
