[ https://issues.apache.org/jira/browse/HDFS-1073?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13070745#comment-13070745 ]
Todd Lipcon commented on HDFS-1073: ----------------------------------- Committed a number of small fixes to the branch to address the following from Konstantin's review: bq. EditLogOutputStream does not extend OutputStream, what is the reason for that? EditLogOutputStream is moving towards being more like a "sink for journal records" rather than a straight output stream. That is to say, the abstraction is a sequence of edits, rather than a sequence of bytes. So extending OutputStream wasn't really buying us anything. Good point about the javadoc inheritance. I added JavaDoc now that it it is its own class. I also removed the {{write(int)}} API which was only used internally. bq. BackupImage.BNState - convert field descriptions to JavaDoc. Fixed. bq. BackupNodeProtocol would it be better to call it JournalProtocol Since it's only used for transferring edits to the BackupNode (and not any other type of journaling) I think the current name makes more sense. It's also more consistent with the other protocols like DatanodeProtocol and NamenodeProtocol. bq. In FSEditLogOpCodes comment below refers to a non existing entity. Probably redundant... Fixed. I also noticed that OP_JSPOOL_START was referenced in the code in a few places, but no longer necessary. I removed it as well as the isOperationSupported() call which was no longer necessary. bq. JournalManager, BackupJournalManager, FileJournalManager should not be public. BackupJournalManager needs JavaDoc. Fixed. bq. FSImageOldStorageInspector: "Old" is not informative. Could be something like PreTransactional or Plain or something. Good idea. I renamed it to PreTransactional. bq. Good design doc, but somewhat outdated. Do you plan to update it some time? I see the main purpose of the design doc to guide development, rather than to document the architecture after it's done. I will try to update any places where it's grossly inaccurate after we've merged this to trunk, though. bq. FSEditLog.JournalAndStream should not have public methods if possible. Fixed. {quote} I propose to wrap the part of doCheckpoint() that transfers image and edits from NN in downloadCheckpoint() method to make the former more readable. Also I recommend first downloading all files (image and edits), then applying them to memory. Now you do: download image, apply image, download edits, apply edits. Should be: download, download, apply, apply. That way it will fail fast if download is not successful. {quote} I looked into doing this, but it wasn't straightforward, since we don't always need to download the image. Would it be alright to address this after the merge? I can file a JIRA so it doesn't get forgotten. bq. When a stream gets bad, we should force syncing remaining journal streams, don't we? Otherwise there is no way to distinguish between failed streams and the valid ones. Or did I miss something? I'm not sure I follow. We only detect that a stream is bad when we're syncing, so we're syncing the other ones at the same time anyway. We know that stream is bad because {{JournalAndStream.isActive}} returns false after the stream is aborted. On disk, we can distinguish it from the non-failed streams since the non-failed streams will be longer during log recovery. See {{TestFSImageStorageInspector.testLogGroupRecoveryInProgress}} as well as some of the crash-recovery related test cases in {{TestFSEditLog}}. bq. TransferFsImage should send/receive CheckpointSignature as a parameter to make sure that requests belong to the valid checkpoint We do validate that the request belongs to the correct namespace by passing the namespaceID/clusterID/etc. See GetImageServlet.java:92 and {{TestCheckpoint.testReformatNNBetweenCheckpoints}}. > Simpler model for Namenode's fs Image and edit Logs > ---------------------------------------------------- > > Key: HDFS-1073 > URL: https://issues.apache.org/jira/browse/HDFS-1073 > Project: Hadoop HDFS > Issue Type: Improvement > Affects Versions: 0.23.0 > Reporter: Sanjay Radia > Assignee: Todd Lipcon > Fix For: 0.23.0 > > Attachments: hdfs-1073-editloading-algos.txt, hdfs-1073-merge.patch, > hdfs-1073-merge.patch, hdfs-1073.txt, hdfs1073.pdf, hdfs1073.pdf, > hdfs1073.pdf, hdfs1073.tex > > > The naming and handling of NN's fsImage and edit logs can be significantly > improved resulting simpler and more robust code. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira