Details

    • Hadoop Flags:
      Reviewed
    1. h3196_20120417.patch
      30 kB
      Tsz Wo Nicholas Sze
    2. h3196_20120416.patch
      28 kB
      Tsz Wo Nicholas Sze
    3. h3196_20120412.patch
      14 kB
      Tsz Wo Nicholas Sze

      Activity

      Hide
      Tsz Wo Nicholas Sze added a comment -

      h3196_20120412.patch:

      • adds JournalDiskWriter;
      • renames JournalListener.rollLogs(..) to startLogSegment(..).

      Note that the editlog initialization will be done separately.

      There are some code refactoring in the patch. I am think about doing it on trunk first.

      Show
      Tsz Wo Nicholas Sze added a comment - h3196_20120412.patch: adds JournalDiskWriter; renames JournalListener.rollLogs(..) to startLogSegment(..). Note that the editlog initialization will be done separately. There are some code refactoring in the patch. I am think about doing it on trunk first.
      Hide
      Suresh Srinivas added a comment -

      Comments:

      1. JournalDiskWriter
        • Looks like it is incomplete. Please add TODO items at least.
        • When is editlog instantiated, you need directories where editlogs are written etc. Currently editlog is null.
        • Please add tests for JournalDiskWriter.
      2. I had renamed
      3. Unrelated to this patch, can you remove unneccessary IOException thrown frmo FSEditLog#namenodeStartedLogSegment.
      4. I prefer calling namenodeStartedLogSegment to startLogSegment and indicate in the comments that the remote namenode started a new log segment.

      I like the idea of refactoring the code in trunk first and bringing it in.

      Show
      Suresh Srinivas added a comment - Comments: JournalDiskWriter Looks like it is incomplete. Please add TODO items at least. When is editlog instantiated, you need directories where editlogs are written etc. Currently editlog is null. Please add tests for JournalDiskWriter. I had renamed Unrelated to this patch, can you remove unneccessary IOException thrown frmo FSEditLog#namenodeStartedLogSegment. I prefer calling namenodeStartedLogSegment to startLogSegment and indicate in the comments that the remote namenode started a new log segment. I like the idea of refactoring the code in trunk first and bringing it in.
      Hide
      Tsz Wo Nicholas Sze added a comment -

      I created HDFS-3273 for refactoring. Then, I am going to work on editlog initialization here.

      Show
      Tsz Wo Nicholas Sze added a comment - I created HDFS-3273 for refactoring. Then, I am going to work on editlog initialization here.
      Hide
      Tsz Wo Nicholas Sze added a comment -

      Hari's patch in HDFS-3274 has editlog initialization. We should simply use it in JournalDiskWriter.

      Show
      Tsz Wo Nicholas Sze added a comment - Hari's patch in HDFS-3274 has editlog initialization. We should simply use it in JournalDiskWriter.
      Hide
      Tsz Wo Nicholas Sze added a comment -

      h3196_20120416.patch: adds a Journal class so that reader and writer can use it.

      Show
      Tsz Wo Nicholas Sze added a comment - h3196_20120416.patch: adds a Journal class so that reader and writer can use it.
      Hide
      Hari Mankude added a comment -

      Minor comments:

      • JournalService should create the journal object.
      • Please verify that in-memory structures are initialized correctly after the format() command
      Show
      Hari Mankude added a comment - Minor comments: JournalService should create the journal object. Please verify that in-memory structures are initialized correctly after the format() command
      Hide
      Tsz Wo Nicholas Sze added a comment -

      h3196_20120417.patch:

      • removes Journal from the JournalService constructor;
      • adds TestJournal for testing format.
      Show
      Tsz Wo Nicholas Sze added a comment - h3196_20120417.patch: removes Journal from the JournalService constructor; adds TestJournal for testing format.
      Hide
      Hari Mankude added a comment -

      Looks good
      +1

      Show
      Hari Mankude added a comment - Looks good +1
      Hide
      Tsz Wo Nicholas Sze added a comment -

      Thanks Hari and Suresh for reviewing this.

      I have committed this.

      Show
      Tsz Wo Nicholas Sze added a comment - Thanks Hari and Suresh for reviewing this. I have committed this.

        People

        • Assignee:
          Tsz Wo Nicholas Sze
          Reporter:
          Tsz Wo Nicholas Sze
        • Votes:
          0 Vote for this issue
          Watchers:
          1 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development