Details

    • Hadoop Flags:
      Reviewed
    1. hdfs-3213-2.patch
      12 kB
      Hari Mankude
    2. hdfs-3213-1.patch
      11 kB
      Hari Mankude
    3. hdfs-3213.patch
      11 kB
      Hari Mankude

      Activity

      Hide
      Tsz Wo Nicholas Sze added a comment -

      +1 patch looks good

      I have committed this. Thanks, Hari!

      Show
      Tsz Wo Nicholas Sze added a comment - +1 patch looks good I have committed this. Thanks, Hari!
      Hide
      Hari Mankude added a comment -

      JournalService.fence(..), should the setupStorage(..) call be after verifyFence(..) and verify(..)?

      One of the advantages of doing this earlier is that verify() will check for results automatically.

      JournalService.conf is only used in setupStorage(..). Instead of adding it as a field, it is better to replace it with NNStorage since the conf values are fixed. We don't have to create the objects in setupStorage(..).

      Let us leave this as is for now. This will require rework once edits dir format for journals is decided.

      Need to add Assert.fail() after fence(..) in the new tests. Otherwise, the test will pass even if fence(..) does not throw exceptions. I think it is better to print out the exception in the catch-blocks instead of leaving them empty.

      Done for both, assert.fail() and printfs.

      In addition, we need to think about how to synchronize the methods in JournalService. Do you want to do it here or in a separated issue?

      Again, I would like to defer this for later. I will file a jira for this.

      Show
      Hari Mankude added a comment - JournalService.fence(..), should the setupStorage(..) call be after verifyFence(..) and verify(..)? One of the advantages of doing this earlier is that verify() will check for results automatically. JournalService.conf is only used in setupStorage(..). Instead of adding it as a field, it is better to replace it with NNStorage since the conf values are fixed. We don't have to create the objects in setupStorage(..). Let us leave this as is for now. This will require rework once edits dir format for journals is decided. Need to add Assert.fail() after fence(..) in the new tests. Otherwise, the test will pass even if fence(..) does not throw exceptions. I think it is better to print out the exception in the catch-blocks instead of leaving them empty. Done for both, assert.fail() and printfs. In addition, we need to think about how to synchronize the methods in JournalService. Do you want to do it here or in a separated issue? Again, I would like to defer this for later. I will file a jira for this.
      Hide
      Tsz Wo Nicholas Sze added a comment -
      • In JournalService.fence(..), should the setupStorage(..) call be after verifyFence(..) and verify(..)?
      • JournalService.conf is only used in setupStorage(..). Instead of adding it as a field, it is better to replace it with NNStorage since the conf values are fixed. We don't have to create the objects in setupStorage(..).
      • Need to add Assert.fail() after fence(..) in the new tests. Otherwise, the test will pass even if fence(..) does not throw exceptions. I think it is better to print out the exception in the catch-blocks instead of leaving them empty.

      In addition, we need to think about how to synchronize the methods in JournalService. Do you want to do it here or in a separated issue?

      Show
      Tsz Wo Nicholas Sze added a comment - In JournalService.fence(..), should the setupStorage(..) call be after verifyFence(..) and verify(..)? JournalService.conf is only used in setupStorage(..). Instead of adding it as a field, it is better to replace it with NNStorage since the conf values are fixed. We don't have to create the objects in setupStorage(..). Need to add Assert.fail() after fence(..) in the new tests. Otherwise, the test will pass even if fence(..) does not throw exceptions. I think it is better to print out the exception in the catch-blocks instead of leaving them empty. In addition, we need to think about how to synchronize the methods in JournalService. Do you want to do it here or in a separated issue?
      Hide
      Hari Mankude added a comment -

      Attached a patch for branch-3092. Not sure if jenkins run can be started on the branch.

      Show
      Hari Mankude added a comment - Attached a patch for branch-3092. Not sure if jenkins run can be started on the branch.
      Hide
      Hari Mankude added a comment -

      Yep, that is the plan.

      Show
      Hari Mankude added a comment - Yep, that is the plan.
      Hide
      Todd Lipcon added a comment -

      I'm assuming you'll use StorageDirectory here, which will take care of this all for you, right?

      Show
      Todd Lipcon added a comment - I'm assuming you'll use StorageDirectory here, which will take care of this all for you, right?
      Hide
      Hari Mankude added a comment -

      Journal Daemon should persist nsid and clusterid in the local directory. The first fence is used to get the cluster information. This information has to be persisted in the edits dir so that the journal daemon functioning off this storage directory is used to talk to only one cluster.

      Show
      Hari Mankude added a comment - Journal Daemon should persist nsid and clusterid in the local directory. The first fence is used to get the cluster information. This information has to be persisted in the edits dir so that the journal daemon functioning off this storage directory is used to talk to only one cluster.

        People

        • Assignee:
          Hari Mankude
          Reporter:
          Hari Mankude
        • Votes:
          0 Vote for this issue
          Watchers:
          2 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development