hadoop-hdfs-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Todd Lipcon" <t...@apache.org>
Subject Re: Review Request: HDFS-1580: Add interface for generic Write Ahead Logging mechanisms
Date Wed, 02 Nov 2011 17:21:00 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2672/#review3014
-----------------------------------------------------------



hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
<https://reviews.apache.org/r/2672/#comment6727>

    we use _PREFIX instead of _BASE elsewhere for key prefixes



hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
<https://reviews.apache.org/r/2672/#comment6728>

    why not just use conf.getClass here and return a Class? And throw the exception right
here instead of returning null and throwing below



hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeResourceChecker.java
<https://reviews.apache.org/r/2672/#comment6729>

    this is the wrong layer - better to filter for file:// URLs where this is called, I think.



hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGenericJournalConf.java
<https://reviews.apache.org/r/2672/#comment6730>

    no need to have any datanodes for any of these tests - will run faster without.



hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGenericJournalConf.java
<https://reviews.apache.org/r/2672/#comment6731>

    our convention is to use american spelling (initialized)



hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGenericJournalConf.java
<https://reviews.apache.org/r/2672/#comment6732>

    our style is to not have multiple classes per .java file unless they're inner classes.
You can make this a static inner class of the test.



hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGenericJournalConf.java
<https://reviews.apache.org/r/2672/#comment6733>

    just return Mockito.mock(EditLogOutputStream.class) and you don't need to have the whole
implementation below


- Todd


On 2011-11-02 14:33:47, Ivan Kelly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/2672/
> -----------------------------------------------------------
> 
> (Updated 2011-11-02 14:33:47)
> 
> 
> Review request for hadoop-hdfs.
> 
> 
> Summary
> -------
> 
> This is the final piece to allow the loading of custom implementations of JournalManager.
There is another change HDFS-2334 which adds closeable to JournalManager, but that may not
be absolutely necessary for all journal types. (it is for bookkeeper)
> 
> There's 2 changes:
> 1) I've changes the interfaces(JournalManager, EditLogInputStream & EditLogOutputStream)
so that they can be implemented outside of the org.apache.hadoop.hdfs.server.namenode.
> 
> 2) Pluggable creation of journal managers.
> When FSEditLog is creating JournalManagers from dfs.namenode.edits.dir, and it encounters
a URI with a schema different to "file" it loads the name of the implementing class from "dfs.namenode.edits.journal-plugin.<schema>".
This class must implement JournalManager and have a constructor which takes (Configuration,
URI).
> 
> 
> This addresses bug HDFS-1580.
>     http://issues.apache.org/jira/browse/HDFS-1580
> 
> 
> Diffs
> -----
> 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
dd39676 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogBackupInputStream.java
974697d 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogBackupOutputStream.java
067990d 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileInputStream.java
9db7f8a 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogFileOutputStream.java
4780d04 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogInputStream.java
c6f8505 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EditLogOutputStream.java
8681837 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
f80f863 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
991fd08 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java
3adb439 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/JournalManager.java
348e3ef 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/JournalSet.java
45b5714 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NNStorage.java
a7fa7fb 
>   hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeResourceChecker.java
4d7cfd8 
>   hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestGenericJournalConf.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/2672/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ivan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message