Mailing-List: contact hdfs-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: hdfs-issues@hadoop.apache.org
Date: Mon, 29 Oct 2012 23:46:14 +0000 (UTC)
From: "Todd Lipcon (JIRA)" <jira@apache.org>
To: hdfs-issues@hadoop.apache.org
Message-ID: <2126420447.41782.1351554374743.JavaMail.jiratomcat@arcas>
In-Reply-To: <1445457911.36422.1350026343113.JavaMail.jiratomcat@arcas>
Subject: [jira] [Commented] (HDFS-4045) SecondaryNameNode cannot read from
 QuorumJournal URI
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/HDFS-4045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13486510#comment-13486510 ] 

Todd Lipcon commented on HDFS-4045:
-----------------------------------

{code}
+  @Override
+  public InputStream getInputStream() throws IOException {
+    throw new NotImplementedException();
+  }
+
{code}

We generally don't use this class from commons-lang. Instead, UnsupportedOperationException is probably to be preferred. This is in a few places. Should we also add TODOs here?
----

Tests:

- The two test cases are identical except that one of them has both a local edits dir and a QJM configured, whereas the second has only QJM, right? Instead of duplicating all the code, can you make them both call through to a method with a {{boolean alsoIncludeLocalEdits}} or something? That would make it much easier to tell the difference between the two cases, and cleaner code.

- Also, can you add some assertions that a new checkpoint is actually uploaded by the 2NN to the NN? ie that it successfully checkpointed something instead of being a no-op?

----
Nits:

- A bunch of changes in TestFileJournalManager don't seem to be necessary for this patch: why switch out FileJournalManager for JournalManager?
- Why is the test called TestSnapshotWithQJM? It probably should be TestCheckpointWithQJM -- otherwise it looks related to HDFS Snapshots
- Some indentation is off in testSecondaryCheckpoint
- the new startSecondaryNameNode function in TestNNWithQJM doesn't seem to be used. There are also some new unused imports. In general, please try to review the patch files so that they only make the minimum changes to fix the issue at hand.
- {{getStreamServer}} seems to share most of its code with {{getFileServer}}. Can you refactor them to share code? I'd also pick a different name, like {{copyInputStreamToResponse}} or {{streamServletOutput}} or something since {{get*}} should be reserved for functions that return a value.

----
General thought:
I'm not wild about exposing the underlying input stream via {{getInputStream}}, for a few reasons:

1) It means that we don't get the benefit of the RedundantInputStream stuff that Colin did -- if there's an error in the middle of serving a stream, we won't fail over to another option.
2) It seems like the {{serveEdits}} API that you added to JournalManager overlaps quite a bit with {{getInputStream}}.

Instead, what about adding only {{serveEdits}}, and adding {{getInputStream}} as necessary to the JM-specific subclasses of EditLogInputStream where appropriate? This makes more sense because, for example in the case of QJM, we could have serveEdits issue a 302 redirect to one of the JournalNodes and avoid the 'proxying' through the NN. It also maintains the abstraction of EditLogInputStream only being an iterator for edit log ops, and not a bytewise interface.

                
> SecondaryNameNode cannot read from QuorumJournal URI
> ----------------------------------------------------
>
>                 Key: HDFS-4045
>                 URL: https://issues.apache.org/jira/browse/HDFS-4045
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: name-node
>    Affects Versions: 3.0.0
>            Reporter: Vinithra Varadharajan
>            Assignee: Andy Isaacson
>         Attachments: hdfs-4045-2.txt, hdfs4045-3.txt, hdfs-4045.txt
>
>
> If HDFS is set up in basic mode (non-HA) with QuorumJournal, and the dfs.namenode.edits.dir is set to only the QuorumJournal URI and no local dir, the SecondaryNameNode is unable to do a checkpoint.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira