Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Duplicate
    • Affects Version/s: HA branch (HDFS-1623)
    • Fix Version/s: HA branch (HDFS-1623)
    • Component/s: ha, test
    • Labels:
      None

      Description

      Looks like it's passing a null nameservice ID, which I believe indicates a non-HA setup.

        Activity

        Hide
        atm Aaron T. Myers added a comment -

        Here's a patch which addresses the issue.

        Show
        atm Aaron T. Myers added a comment - Here's a patch which addresses the issue.
        Hide
        tlipcon Todd Lipcon added a comment -

        +1

        Show
        tlipcon Todd Lipcon added a comment - +1
        Hide
        umamaheswararao Uma Maheswara Rao G added a comment -

        Thanks Aaron for filing the JIRA. forgot to add TODO while setting up the basic balancer setup ready with HA. Using MiniDFSNNTopology.simpleHATopology(), it should work. But the current problem is NameNodeConnector uses real NN uri for creating the failover proxy. This test sets the deafult port, so it is able to get the proxy provider. Balancer should work with any address, HDFS-2979 should address the issue.

        Show
        umamaheswararao Uma Maheswara Rao G added a comment - Thanks Aaron for filing the JIRA. forgot to add TODO while setting up the basic balancer setup ready with HA. Using MiniDFSNNTopology.simpleHATopology(), it should work. But the current problem is NameNodeConnector uses real NN uri for creating the failover proxy. This test sets the deafult port, so it is able to get the proxy provider. Balancer should work with any address, HDFS-2979 should address the issue.
        Hide
        tlipcon Todd Lipcon added a comment -

        Uma: I didn't quite follow your comment. Do you think this patch is unnecessary/incorrect? Or just pointing out another problem in the same area of the code? Should we commit this?

        Show
        tlipcon Todd Lipcon added a comment - Uma: I didn't quite follow your comment. Do you think this patch is unnecessary/incorrect? Or just pointing out another problem in the same area of the code? Should we commit this?
        Hide
        umamaheswararao Uma Maheswara Rao G added a comment -

        This issue is a valid point for setting nsId. But actually we need not set the nsid explicitly in test code as MiniDFSNNTopology.simpleHATopology() can set nsID if we use it directly. So, why can't we use it?
        Currently can not use directly MiniDFSNNTopology.simpleHATopology(), because of HDFS-2979. After addressing HDFS-2979, we can directly use MiniDFSNNTopology.simpleHATopology(), obviously with that change this test will not fail.

        just pointing out another problem in the same area of the code?

        Yes, other problem in source.

        Should we commit this?

        Could you please take a look before committing this.

        Show
        umamaheswararao Uma Maheswara Rao G added a comment - This issue is a valid point for setting nsId. But actually we need not set the nsid explicitly in test code as MiniDFSNNTopology.simpleHATopology() can set nsID if we use it directly. So, why can't we use it? Currently can not use directly MiniDFSNNTopology.simpleHATopology(), because of HDFS-2979 . After addressing HDFS-2979 , we can directly use MiniDFSNNTopology.simpleHATopology(), obviously with that change this test will not fail. just pointing out another problem in the same area of the code? Yes, other problem in source. Should we commit this? Could you please take a look before committing this.
        Hide
        atm Aaron T. Myers added a comment -

        Though it's not a perfect duplicate, this issue will be resolved incidentally by HDFS-2979. So, I'll close this JIRA and go review that JIRA.

        Thanks a lot for looking into this, Uma.

        Show
        atm Aaron T. Myers added a comment - Though it's not a perfect duplicate, this issue will be resolved incidentally by HDFS-2979 . So, I'll close this JIRA and go review that JIRA. Thanks a lot for looking into this, Uma.

          People

          • Assignee:
            atm Aaron T. Myers
            Reporter:
            atm Aaron T. Myers
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development