HBase
  1. HBase
  2. HBASE-7840

Enhance the java it framework to start & stop a distributed hbase & hadoop cluster

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 0.95.2
    • Fix Version/s: 0.99.0
    • Component/s: test
    • Labels:
      None

      Description

      Needs are to use a development version of HBase & HDFS 1 & 2.

      Ideally, should be nicely backportable to 0.94 to allow comparisons and regression tests between versions.

      1. 7840.v1.patch
        55 kB
        Nicolas Liochon
      2. 7840.v3.patch
        63 kB
        Nicolas Liochon
      3. 7840.v4.patch
        84 kB
        Nicolas Liochon

        Issue Links

          Activity

          Hide
          Nicolas Liochon added a comment -

          Too old.

          Show
          Nicolas Liochon added a comment - Too old.
          Hide
          Nick Dimiduk added a comment -

          Or can we test for SUDO and have the test 'pass' if not present w/ WARNING or something that test did not run?

          I wonder if you could use the Parameterized @Test to detect sudo privileges and execute accordingly.

          Show
          Nick Dimiduk added a comment - Or can we test for SUDO and have the test 'pass' if not present w/ WARNING or something that test did not run? I wonder if you could use the Parameterized @Test to detect sudo privileges and execute accordingly.
          Hide
          Nicolas Liochon added a comment -

          do you think it reasonable to expect that hbase user have sudo on a test rig

          I think it's reasonable to ask for it. I will see what I can do. But i will need to commit something sometimes

          Show
          Nicolas Liochon added a comment - do you think it reasonable to expect that hbase user have sudo on a test rig I think it's reasonable to ask for it. I will see what I can do. But i will need to commit something sometimes
          Hide
          stack added a comment -

          Hmmm.... It will mess up our green lights on test rigs? Or do you think it reasonable to expect that hbase user have sudo on a test rig. Can this test be off by default? Would that be an option? Or can we test for SUDO and have the test 'pass' if not present w/ WARNING or something that test did not run?

          Show
          stack added a comment - Hmmm.... It will mess up our green lights on test rigs? Or do you think it reasonable to expect that hbase user have sudo on a test rig. Can this test be off by default? Would that be an option? Or can we test for SUDO and have the test 'pass' if not present w/ WARNING or something that test did not run?
          Hide
          Nicolas Liochon added a comment -

          I'd say don't commit unless all pass

          ok.

          Do I need to have sudo perms to now run hbase-it

          For the one in mttr, yes. The test will fail (not nicely, it will fail immediately).

          Show
          Nicolas Liochon added a comment - I'd say don't commit unless all pass ok. Do I need to have sudo perms to now run hbase-it For the one in mttr, yes. The test will fail (not nicely, it will fail immediately).
          Hide
          stack added a comment -

          I'd say don't commit unless all pass. Do I need to have sudo perms to now run hbase-it? Or the tests just fail if I don't have sudo?

          Show
          stack added a comment - I'd say don't commit unless all pass. Do I need to have sudo perms to now run hbase-it? Or the tests just fail if I don't have sudo?
          Hide
          Nicolas Liochon added a comment -

          Thanks a lot for the reviews.

          What is this comment in ClusterManager.java?

          I wanted it to be in the parent class, but it's not possible as it depends on methods available in the subclass only

          Bit odd this is all hardcoded into the script:

          I've made the heap size configurable. Yeah this part is not perfect.

          Do they work?

          All the tests that are in this patch work. I've got a few that don't. I'm fixing them (it's could be HBase core code: they used to work).

          I will push this version soon.

          Show
          Nicolas Liochon added a comment - Thanks a lot for the reviews. What is this comment in ClusterManager.java? I wanted it to be in the parent class, but it's not possible as it depends on methods available in the subclass only Bit odd this is all hardcoded into the script: I've made the heap size configurable. Yeah this part is not perfect. Do they work? All the tests that are in this patch work. I've got a few that don't. I'm fixing them (it's could be HBase core code: they used to work). I will push this version soon.
          Hide
          stack added a comment -

          What is this comment in ClusterManager.java?

          +  // Not in ClusterManager because it uses 'exec'
          

          Bit odd this is all hardcoded into the script:

          +      cmd += "export HBASE_REGIONSERVER_OPTS='" +
          +          jmx +
          +          "-server -XX:ParallelGCThreads=4 -XX:+UseParNewGC -Xmn512m " +
          +          "-XX:CMSInitiatingOccupancyFraction=80 -verbose:gc -XX:+PrintGCDetails " +
          +          "-XX:PrintFLSStatistics=1  -XX:+PrintPromotionFailure " +
          +          "-XX:+PrintGCTimeStamps -XX:+PrintGCDateStamps -Xloggc:" +
          +          getHBaseHome() + "/logs/gc-" + service + ".log " +
          +          "-Xms" + heapSize + "m -Xmx" + heapSize + "m -XX:ErrorFile=" +
          +          getHBaseHome() + "/logs/gc-" + service + "-err.log" +
          +          "'; ";
          

          Woah!!! Rad:

          +    toExec += "sudo /sbin/iptables -A INPUT -i lo -j ACCEPT;";
          +    toExec += "sudo /sbin/iptables -A OUTPUT -o lo -j ACCEPT;";
          +    toExec += "sudo /sbin/iptables -A INPUT -p tcp -s 0/0 -d " + hostname +
          +        " --sport 513:65535 --dport 22 -m state --state NEW,ESTABLISHED -j ACCEPT;";
          +    toExec += "sudo /sbin/iptables -A OUTPUT -p tcp -s " + hostname +
          +        " -d 0/0 --sport 22 --dport 513:65535 -m state --state ESTABLISHED -j ACCEPT;";
          +    toExec += "sudo /sbin/iptables -P INPUT DROP;";
          +    toExec += "sudo /sbin/iptables -P OUTPUT DROP;";
          +    toExec += "sudo /sbin/iptables -P FORWARD DROP;";
          

          I skimmed the patch. These are some really great tests. Do they work? +1 on commit if they do (even if above freaks me out – smile)

          Show
          stack added a comment - What is this comment in ClusterManager.java? + // Not in ClusterManager because it uses 'exec' Bit odd this is all hardcoded into the script: + cmd += "export HBASE_REGIONSERVER_OPTS='" + + jmx + + "-server -XX:ParallelGCThreads=4 -XX:+UseParNewGC -Xmn512m " + + "-XX:CMSInitiatingOccupancyFraction=80 -verbose:gc -XX:+PrintGCDetails " + + "-XX:PrintFLSStatistics=1 -XX:+PrintPromotionFailure " + + "-XX:+PrintGCTimeStamps -XX:+PrintGCDateStamps -Xloggc:" + + getHBaseHome() + "/logs/gc-" + service + ".log " + + "-Xms" + heapSize + "m -Xmx" + heapSize + "m -XX:ErrorFile=" + + getHBaseHome() + "/logs/gc-" + service + "-err.log" + + "'; " ; Woah!!! Rad: + toExec += "sudo /sbin/iptables -A INPUT -i lo -j ACCEPT;" ; + toExec += "sudo /sbin/iptables -A OUTPUT -o lo -j ACCEPT;" ; + toExec += "sudo /sbin/iptables -A INPUT -p tcp -s 0/0 -d " + hostname + + " --sport 513:65535 --dport 22 -m state --state NEW,ESTABLISHED -j ACCEPT;" ; + toExec += "sudo /sbin/iptables -A OUTPUT -p tcp -s " + hostname + + " -d 0/0 --sport 22 --dport 513:65535 -m state --state ESTABLISHED -j ACCEPT;" ; + toExec += "sudo /sbin/iptables -P INPUT DROP;" ; + toExec += "sudo /sbin/iptables -P OUTPUT DROP;" ; + toExec += "sudo /sbin/iptables -P FORWARD DROP;" ; I skimmed the patch. These are some really great tests. Do they work? +1 on commit if they do (even if above freaks me out – smile)
          Hide
          Nick Dimiduk added a comment -

          +1 on latest patch. This is good work, Nicolas.

          Show
          Nick Dimiduk added a comment - +1 on latest patch. This is good work, Nicolas.
          Hide
          Nicolas Liochon added a comment -

          New version.

          Show
          Nicolas Liochon added a comment - New version.
          Hide
          Nicolas Liochon added a comment -

          My local version has evolved since my initial submission. I need to work a little on it to make interesting for everybody....

          Show
          Nicolas Liochon added a comment - My local version has evolved since my initial submission. I need to work a little on it to make interesting for everybody....
          Hide
          stack added a comment -

          Skimmed the patch. Looks like nice new test functionality. If it works I'm good w/ commit.

          Show
          stack added a comment - Skimmed the patch. Looks like nice new test functionality. If it works I'm good w/ commit.
          Hide
          Nicolas Liochon added a comment -

          Waiting for review or +1 on this one, I can rebase if you like.

          Show
          Nicolas Liochon added a comment - Waiting for review or +1 on this one, I can rebase if you like.
          Hide
          Nicolas Liochon added a comment -

          v3 on rb as well, rebased.

          Show
          Nicolas Liochon added a comment - v3 on rb as well, rebased.
          Hide
          Nicolas Liochon added a comment -

          I've put the v2 in the review boards a few days ago, awaiting feedback. I can rebase if necessary.

          Show
          Nicolas Liochon added a comment - I've put the v2 in the review boards a few days ago, awaiting feedback. I can rebase if necessary.
          Hide
          Nicolas Liochon added a comment -
          Show
          Nicolas Liochon added a comment - done: https://reviews.apache.org/r/9437/
          Hide
          Enis Soztutar added a comment -

          Patch looks promising. Can you please put that in RB. I'll take a closer look at it soon.

          Show
          Enis Soztutar added a comment - Patch looks promising. Can you please put that in RB. I'll take a closer look at it soon.
          Hide
          Nicolas Liochon added a comment -

          It's not perfect, but considering the subject it's in a reviewable state imho. Or at least deserves some high level comments

          Show
          Nicolas Liochon added a comment - It's not perfect, but considering the subject it's in a reviewable state imho. Or at least deserves some high level comments

            People

            • Assignee:
              Nicolas Liochon
              Reporter:
              Nicolas Liochon
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development