Bigtop
  1. Bigtop
  2. BIGTOP-732

Support running multiple HBase region servers

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.6.0
    • Component/s: None
    • Labels:
      None

      Description

      Previously on the mailing list I submitted the idea of supporting multiple region server daemons on the same system. This can be done using the local-regionservers.sh and local-masters.sh scripts that we remove from our packaging (see BIGTOP-503), but apparently running multiple region servers in production can be useful. It should be possible through init scripts, and it should play nice with the more traditional use case.

      The modified init script template should make it safe and intuitive to run multiple region servers and masters on the same system, but only the regionserver package is using the new template - I don't know of a good reason to run multiple masters in production. Using the init script as before will control a single region-server daemon EXACTLY as it did before. If you specify numbers as additional parameters, you can control multiple daemons:

       
      service hbase-regionserver start # Starts a single region server daemon, as before
      (all other commands, with no additional parameters, will work as before)
      
      service hbase-regionserver start 1 2 3 4 # Starts a single region server daemon
      service hbase-regionserver restart 2 4 # Restarts the even daemons
      service hbase-regionserver stop 1 3 # Stops the odd daemons
      service hbase-regionserver stop # Stops all region servers in any mode of operation
      service hbase-regionserver restart 1 2 3 # Stops all region servers, then starts these 3
      

      I can see a case being made for changing the behavior of the stop and restart commands - so let me know if you disagree with the path I took. The log files and pid files get put in the same directory, but are also numbered according to their offset. The force-stop and force-reload command should also work as expected. When running a single daemon you can't start multiple daemons, and vice-versa. As recommended by Bruno for LSB-compliance and ease of administration, you can specify the offsets in /etc/hbase/conf/regionserver_offsets instead of on the command-line. Specifying offsets on the command-line anyway will override the file.

      1. 0001-BIGTOP-732.-Support-running-multiple-HBase-region-se.patch
        18 kB
        Sean Mackrory
      2. 0001-BIGTOP-732.-Support-running-multiple-HBase-region-se.patch
        17 kB
        Sean Mackrory
      3. BIGTOP-732.patch.5
        17 kB
        Sean Mackrory
      4. BIGTOP-732.patch.4
        17 kB
        Sean Mackrory
      5. BIGTOP-732.patch.3
        16 kB
        Sean Mackrory
      6. BIGTOP-732.patch.2
        16 kB
        Sean Mackrory
      7. BIGTOP-732.patch.1
        15 kB
        Sean Mackrory

        Issue Links

          Activity

          Hide
          Sean Mackrory added a comment -

          This patch was based on the previous script used for DEB packages, but I've tested it on Ubuntu 10.04, CentOS 6, and OpenSuSE 12.

          Show
          Sean Mackrory added a comment - This patch was based on the previous script used for DEB packages, but I've tested it on Ubuntu 10.04, CentOS 6, and OpenSuSE 12.
          Hide
          Jonathan Hsieh added a comment -

          Looks reasonable.

          Looks like 'service hbase-regionserver status' dumps for each "offset"?

          Is there a pointer to how I can try this?

          Show
          Jonathan Hsieh added a comment - Looks reasonable. Looks like 'service hbase-regionserver status' dumps for each "offset"? Is there a pointer to how I can try this?
          Hide
          Sean Mackrory added a comment - - edited

          I've updated the patch as the old one had bit-rotted and no-longer applied cleanly.

          Show
          Sean Mackrory added a comment - - edited I've updated the patch as the old one had bit-rotted and no-longer applied cleanly.
          Hide
          Sean Mackrory added a comment -

          Jonathan Hsieh: Apply the patch, build HBase, and deploy. You can try the commands in the code box in the description. You can also put the offsets in the file /etc/hbase/regionserver_offsets and have them provided automatically every time.

          Show
          Sean Mackrory added a comment - Jonathan Hsieh : Apply the patch, build HBase, and deploy. You can try the commands in the code box in the description. You can also put the offsets in the file /etc/hbase/regionserver_offsets and have them provided automatically every time.
          Hide
          Jonathan Hsieh added a comment -

          We need some stronger input validation checks along with comments at the header of the file on how to use this.

          This is not polished

          ~# service hbase-regionserver start foo bar baz
          /etc/init.d/hbase-regionserver: line 165: [: foo: integer expression expected
          Starting regionserver daemon foo: expr: non-integer argument
          expr: non-integer argument
          OK
          /etc/init.d/hbase-regionserver: line 165: [: bar: integer expression expected
          Starting regionserver daemon bar: expr: non-integer argument
          expr: non-integer argument
          OK
          /etc/init.d/hbase-regionserver: line 165: [: baz: integer expression expected
          Starting regionserver daemon baz: expr: non-integer argument
          expr: non-integer argument
          OK
          ~# 
          

          It also leaves some junk behind.

          root@bigtop-732:~# service hbase-regionserver status
          HBase regionserver 1: running
          HBase regionserver 2: running
          HBase regionserver 3: running
          HBase regionserver 4: running
          HBase regionserver bar: not running
          HBase regionserver baz: not running
          HBase regionserver foo: not running
          
          Show
          Jonathan Hsieh added a comment - We need some stronger input validation checks along with comments at the header of the file on how to use this. This is not polished ~# service hbase-regionserver start foo bar baz /etc/init.d/hbase-regionserver: line 165: [: foo: integer expression expected Starting regionserver daemon foo: expr: non-integer argument expr: non-integer argument OK /etc/init.d/hbase-regionserver: line 165: [: bar: integer expression expected Starting regionserver daemon bar: expr: non-integer argument expr: non-integer argument OK /etc/init.d/hbase-regionserver: line 165: [: baz: integer expression expected Starting regionserver daemon baz: expr: non-integer argument expr: non-integer argument OK ~# It also leaves some junk behind. root@bigtop-732:~# service hbase-regionserver status HBase regionserver 1: running HBase regionserver 2: running HBase regionserver 3: running HBase regionserver 4: running HBase regionserver bar: not running HBase regionserver baz: not running HBase regionserver foo: not running
          Hide
          Sean Mackrory added a comment -

          I've added stronger input validation checks and comments in the header of the file on how to use this. I also did some refactoring to make this much cleaner. It's a lot easier to read the code now and make sure that it's behavior is consistent. Did some more thorough testing and fixed a couple of edges cases...

          Show
          Sean Mackrory added a comment - I've added stronger input validation checks and comments in the header of the file on how to use this. I also did some refactoring to make this much cleaner. It's a lot easier to read the code now and make sure that it's behavior is consistent. Did some more thorough testing and fixed a couple of edges cases...
          Hide
          Jonathan Hsieh added a comment -

          Hey Sean, I was meaner this time and got some new funny results.

          root@bigtop-732:~# service hbase-regionserver start 03
          Starting regionserver daemon 03: OK
          root@bigtop-732:~# service hbase-regionserver status
          HBase regionserver 03: not running
          HBase regionserver 3: running
          root@bigtop-732:~# service hbase-regionserver start -1
          Starting regionserver daemon -1: OK
          root@bigtop-732:~# service hbase-regionserver start -0
          Starting regionserver daemon -0: OK
          root@bigtop-732:~# service hbase-regionserver status
          HBase regionserver 03: not running
          HBase regionserver 3: running
          root@bigtop-732:~# jps
          4946 NameNode
          5055 SecondaryNameNode
          5944 HMaster
          4543 NodeManager
          16252 HRegionServer
          5866 QuorumPeerMain
          4850 DataNode
          4680 JobHistoryServer
          16866 HRegionServer
          17005 HRegionServer
          4289 ResourceManager
          17215 Jps
          root@bigtop-732:~# service hbase-regionserver status
          HBase regionserver 03: not running
          HBase regionserver 3: running
          root@bigtop-732:~# 
          
          Show
          Jonathan Hsieh added a comment - Hey Sean, I was meaner this time and got some new funny results. root@bigtop-732:~# service hbase-regionserver start 03 Starting regionserver daemon 03: OK root@bigtop-732:~# service hbase-regionserver status HBase regionserver 03: not running HBase regionserver 3: running root@bigtop-732:~# service hbase-regionserver start -1 Starting regionserver daemon -1: OK root@bigtop-732:~# service hbase-regionserver start -0 Starting regionserver daemon -0: OK root@bigtop-732:~# service hbase-regionserver status HBase regionserver 03: not running HBase regionserver 3: running root@bigtop-732:~# jps 4946 NameNode 5055 SecondaryNameNode 5944 HMaster 4543 NodeManager 16252 HRegionServer 5866 QuorumPeerMain 4850 DataNode 4680 JobHistoryServer 16866 HRegionServer 17005 HRegionServer 4289 ResourceManager 17215 Jps root@bigtop-732:~# service hbase-regionserver status HBase regionserver 03: not running HBase regionserver 3: running root@bigtop-732:~#
          Hide
          Sean Mackrory added a comment - - edited

          Added much more complete validation, and refactored all the validation to occur at the beginning, so no daemons will start unless all of the offsets are valid. In the event that invalid pid files are still created, force-stop will clean them up to stop them from getting in the way of future invocations.

          edit: Specifically, I'm making sure that the offsets consist entirely of digits, have no leading 0's (but can be '0'), are 3 digits or less, and are within the limits of the specific daemon (in this case, between 0 and 100 for the regionserver).

          Show
          Sean Mackrory added a comment - - edited Added much more complete validation, and refactored all the validation to occur at the beginning, so no daemons will start unless all of the offsets are valid. In the event that invalid pid files are still created, force-stop will clean them up to stop them from getting in the way of future invocations. edit: Specifically, I'm making sure that the offsets consist entirely of digits, have no leading 0's (but can be '0'), are 3 digits or less, and are within the limits of the specific daemon (in this case, between 0 and 100 for the regionserver).
          Hide
          Jonathan Hsieh added a comment -

          +1. tested and it works well. skimmed code – nice docs, and looks pretty clean. I didn't look at the packaging specific deb/rpm file edits.

          Show
          Jonathan Hsieh added a comment - +1. tested and it works well. skimmed code – nice docs, and looks pretty clean. I didn't look at the packaging specific deb/rpm file edits.
          Hide
          Roman Shaposhnik added a comment - - edited

          While I'll be reviewing this, just wanted to make a quick comment that /etc/init.d/*_offsets doesn't feels right. Is there any chance offsets can be provided from /etc/default/hbase* ?

          Show
          Roman Shaposhnik added a comment - - edited While I'll be reviewing this, just wanted to make a quick comment that /etc/init.d/* _offsets doesn't feels right. Is there any chance offsets can be provided from /etc/default/hbase * ?
          Hide
          Sean Mackrory added a comment -

          I like that idea much better - done If you have any better ideas for extracting the correct variable on regionserver-init.d.tpl:128-129, I'd love to hear them. I'd rather do it without making the template daemon-specific, just so it can be used for the master if we want to. But the 'eval echo...' seems messy.

          Show
          Sean Mackrory added a comment - I like that idea much better - done If you have any better ideas for extracting the correct variable on regionserver-init.d.tpl:128-129, I'd love to hear them. I'd rather do it without making the template daemon-specific, just so it can be used for the master if we want to. But the 'eval echo...' seems messy.
          Hide
          James Page added a comment -

          Hi Sean

          Roman pinged me for a review of this bug a few weeks ago; apologies for the lag - have been away.

          Anyway - I think the approach to running multiple daemons is just fine; splitting the config into /etc/default/hbase: +1. I can think of several server apps in Ubuntu which already do something quite similar, for example the ceph init script can start multiple daemons based on config (and even across multiple servers - yikes!).

          Something to consider for the future; if/when all distro's deprecate traditional sys v init scripts the single init script currently in use will probably need to be split into multiple <INSERT INIT SYSTEM HERE> configurations. For Ubuntu/upstart, I could see a general start/stop all configurations config and a config that controls/monitors a each instance. But as I said - that's something for the future .

          Show
          James Page added a comment - Hi Sean Roman pinged me for a review of this bug a few weeks ago; apologies for the lag - have been away. Anyway - I think the approach to running multiple daemons is just fine; splitting the config into /etc/default/hbase: +1. I can think of several server apps in Ubuntu which already do something quite similar, for example the ceph init script can start multiple daemons based on config (and even across multiple servers - yikes!). Something to consider for the future; if/when all distro's deprecate traditional sys v init scripts the single init script currently in use will probably need to be split into multiple <INSERT INIT SYSTEM HERE> configurations. For Ubuntu/upstart, I could see a general start/stop all configurations config and a config that controls/monitors a each instance. But as I said - that's something for the future .
          Hide
          Sean Mackrory added a comment -

          James Page - thanks for the feedback! I agree this will need some rethinking when we support SysV init alternatives in the future - I will definitely stay on top of that when the time comes.

          Show
          Sean Mackrory added a comment - James Page - thanks for the feedback! I agree this will need some rethinking when we support SysV init alternatives in the future - I will definitely stay on top of that when the time comes.
          Hide
          Sean Mackrory added a comment -

          Just updating the patch to apply cleanly over BIGTOP-239. I also replaced the use of 'sudo' with 'su'.

          Show
          Sean Mackrory added a comment - Just updating the patch to apply cleanly over BIGTOP-239 . I also replaced the use of 'sudo' with 'su'.
          Hide
          Roman Shaposhnik added a comment -

          Looks good and I really would like to have it for Bigtop 0.6.0. A couple of points though:

          1. it seems that you're only using the new template for the regionservers, yet it has all sorts of 'if [ "@HBASE_DAEMON@" == "master" ] ; then' code blocks. Am I missing something?
          2. we should probably simplify Autodetect JAVA_HOME block since now we have standardized on a common location for bigtop-detect-javahome
          3. The block of code '# Our default HBASE_HOME, HBASE_PID_DIR and HBASE_CONF_DIR' likely belongs to /etc/default/hbase (only then the following if statement is actually useful)
          4. Would it be completely unreasonable to also make FIRST_PORT/FIRST_INFO_PORT tweakable for both types of daemons in /etc/default/hbase (via calling them MASTER_FIRST_PORT, REGIONSERVER_FIRST_PORT, etc)
          5. does the following ever yield false? 'if [ "@HBASE_DAEMON@" != "regionserver" -a "@HBASE_DAEMON@" != "master" ] ; then'

          And my final question – does this patch change the default behaviour/output for the case of a single daemon per node?

          Show
          Roman Shaposhnik added a comment - Looks good and I really would like to have it for Bigtop 0.6.0. A couple of points though: it seems that you're only using the new template for the regionservers, yet it has all sorts of 'if [ "@HBASE_DAEMON@" == "master" ] ; then' code blocks. Am I missing something? we should probably simplify Autodetect JAVA_HOME block since now we have standardized on a common location for bigtop-detect-javahome The block of code '# Our default HBASE_HOME, HBASE_PID_DIR and HBASE_CONF_DIR' likely belongs to /etc/default/hbase (only then the following if statement is actually useful) Would it be completely unreasonable to also make FIRST_PORT/FIRST_INFO_PORT tweakable for both types of daemons in /etc/default/hbase (via calling them MASTER_FIRST_PORT, REGIONSERVER_FIRST_PORT, etc) does the following ever yield false? 'if [ "@HBASE_DAEMON@" != "regionserver" -a "@HBASE_DAEMON@" != "master" ] ; then' And my final question – does this patch change the default behaviour/output for the case of a single daemon per node?
          Hide
          Sean Mackrory added a comment - - edited

          >> it seems that you're only using the new template for the regionservers, yet it has all sorts of 'if [ "@HBASE_DAEMON@" == "master" ] ; then' code blocks. Am I missing something?

          HBase supports doing this with multiple masters as well, so I wanted the template to be flexible. The template is only used for region servers because there are good reasons to colocate multiple region servers in production, but colocating multiple masters is, AFAIK, only useful for testing features like automatic failover in development.

          >> we should probably simplify Autodetect JAVA_HOME block since now we have standardized on a common location for bigtop-detect-javahome

          Good one - will do.

          >> The block of code '# Our default HBASE_HOME, HBASE_PID_DIR and HBASE_CONF_DIR' likely belongs to /etc/default/hbase (only then the following if statement is actually useful)

          Good one - will change...

          >> Would it be completely unreasonable to also make FIRST_PORT/FIRST_INFO_PORT tweakable for both types of daemons in /etc/default/hbase (via calling them MASTER_FIRST_PORT, REGIONSERVER_FIRST_PORT, etc)

          No, I think that's reasonable. Will add.

          >> does the following ever yield false? 'if [ "@HBASE_DAEMON@" != "regionserver" -a "@HBASE_DAEMON@" != "master" ] ; then'

          Only if somebody really goes out of their way to mess stuff up - as I said in #1, the template was written for both regionservers and master, hence the flexibility, but I have no idea how well it works with the other 2 HBase daemons we package.

          >> And my final question – does this patch change the default behaviour/output for the case of a single daemon per node?

          It should not, and in my testing, it does not. I was very careful to make sure that if you invoke the script with only 1 argument (start, stop, etc.) - it only calls the original function - it does not attempt any of the new logic at all.

          Show
          Sean Mackrory added a comment - - edited >> it seems that you're only using the new template for the regionservers, yet it has all sorts of 'if [ "@HBASE_DAEMON@" == "master" ] ; then' code blocks. Am I missing something? HBase supports doing this with multiple masters as well, so I wanted the template to be flexible. The template is only used for region servers because there are good reasons to colocate multiple region servers in production, but colocating multiple masters is, AFAIK, only useful for testing features like automatic failover in development. >> we should probably simplify Autodetect JAVA_HOME block since now we have standardized on a common location for bigtop-detect-javahome Good one - will do. >> The block of code '# Our default HBASE_HOME, HBASE_PID_DIR and HBASE_CONF_DIR' likely belongs to /etc/default/hbase (only then the following if statement is actually useful) Good one - will change... >> Would it be completely unreasonable to also make FIRST_PORT/FIRST_INFO_PORT tweakable for both types of daemons in /etc/default/hbase (via calling them MASTER_FIRST_PORT, REGIONSERVER_FIRST_PORT, etc) No, I think that's reasonable. Will add. >> does the following ever yield false? 'if [ "@HBASE_DAEMON@" != "regionserver" -a "@HBASE_DAEMON@" != "master" ] ; then' Only if somebody really goes out of their way to mess stuff up - as I said in #1, the template was written for both regionservers and master, hence the flexibility, but I have no idea how well it works with the other 2 HBase daemons we package. >> And my final question – does this patch change the default behaviour/output for the case of a single daemon per node? It should not, and in my testing, it does not. I was very careful to make sure that if you invoke the script with only 1 argument (start, stop, etc.) - it only calls the original function - it does not attempt any of the new logic at all.
          Hide
          Roman Shaposhnik added a comment -

          Only if somebody really goes out of their way to mess stuff up - as I said in #1, the template was written for both regionservers and master, hence the flexibility, but I have no idea how well it works with the other 2 HBase daemons we package.

          Wait. This one I don't understand – @HBASE_DAEMON@ is statically replaced during the package build time so there's no chance for something other than regionserver or master to be substituted (unless there's a bug in the package build code, but I'd rather not code for that).

          Show
          Roman Shaposhnik added a comment - Only if somebody really goes out of their way to mess stuff up - as I said in #1, the template was written for both regionservers and master, hence the flexibility, but I have no idea how well it works with the other 2 HBase daemons we package. Wait. This one I don't understand – @HBASE_DAEMON@ is statically replaced during the package build time so there's no chance for something other than regionserver or master to be substituted (unless there's a bug in the package build code, but I'd rather not code for that).
          Hide
          Sean Mackrory added a comment -

          Right - I just didn't want to leave a default case unhandled while I was thinking about it. That is the only reason, so if you'd prefer I can remove the blocks that error out if it's not master or regionserver.

          Show
          Sean Mackrory added a comment - Right - I just didn't want to leave a default case unhandled while I was thinking about it. That is the only reason, so if you'd prefer I can remove the blocks that error out if it's not master or regionserver.
          Hide
          Sean Mackrory added a comment -

          I have:

          • removed the unsupported-daemon cases
          • simplified JAVA_HOME detection
          • honored HBASE_HOME, HBASE_PID_DIR and HBASE_CONF_DIR variables if they are already set (I hesitate to rely on the defaults file because these settings are not supported equally in the other 3 init script templates)
          • Added a comment about why FIRST_PORT / FIRST_INFO_PORT are not configurable by users. Turns out those limits are enforced by HBase itself, so it is not something easily configurable by a user
          • Fixed the pid file cleanup. Previously if regionservers crashed, the presence of the pid files forced some actions to re-use that offset all the time. Now, if you "stop" and already-stopped offset, it deletes the pid file.
          Show
          Sean Mackrory added a comment - I have: removed the unsupported-daemon cases simplified JAVA_HOME detection honored HBASE_HOME, HBASE_PID_DIR and HBASE_CONF_DIR variables if they are already set (I hesitate to rely on the defaults file because these settings are not supported equally in the other 3 init script templates) Added a comment about why FIRST_PORT / FIRST_INFO_PORT are not configurable by users. Turns out those limits are enforced by HBase itself, so it is not something easily configurable by a user Fixed the pid file cleanup. Previously if regionservers crashed, the presence of the pid files forced some actions to re-use that offset all the time. Now, if you "stop" and already-stopped offset, it deletes the pid file.
          Show
          Sean Mackrory added a comment - https://reviews.apache.org/r/10784/
          Hide
          Roman Shaposhnik added a comment -

          +1 and committed!

          Show
          Roman Shaposhnik added a comment - +1 and committed!
          Hide
          Biju Nair added a comment - - edited

          In order to enable monitoring of HBase JMX stats, the JMX port and other JVM parameters need to be set in hbase-env.sh. With support to bring up multiple RS (and masters), what was the thought on how to set the JMX port for the different RS running in a single node?

          It looks like the daemon script may have to be changed since it sources hbase-env.sh.

          Show
          Biju Nair added a comment - - edited In order to enable monitoring of HBase JMX stats, the JMX port and other JVM parameters need to be set in hbase-env.sh . With support to bring up multiple RS (and masters), what was the thought on how to set the JMX port for the different RS running in a single node? It looks like the daemon script may have to be changed since it sources hbase-env.sh .
          Hide
          Andrew Purtell added a comment -

          It looks like we'd need to parameterize the JMX port the same way it's done with others, e.g.:

          	        HBASE_MULTI_ARGS="-D hbase.@HBASE_DAEMON@.port=`expr ${FIRST_PORT} + $OFFSET` \
                                -D hbase.@HBASE_DAEMON@.info.port=`expr ${FIRST_INFO_PORT} + ${OFFSET}`"
          

          Also, the binscripts support pointing them to a configuration directory different than default, so you could do something like:

          su -s /bin/sh hbase -c "$DAEMON_SCRIPT start --conf $DAEMON_HBASE_ETC @HBASE_DAEMON@"
          

          and break out configuration for each daemon instance into /etc/hbase/1/, /etc/hbase/2/, /etc/hbase/3/, ... adjusting hbase-env.sh and hbase-site.xml accordingly in each. It's not as nice as having the service start/stop script do it though, port assignments would need be done by hand.

          Show
          Andrew Purtell added a comment - It looks like we'd need to parameterize the JMX port the same way it's done with others, e.g.: HBASE_MULTI_ARGS="-D hbase.@HBASE_DAEMON@.port=`expr ${FIRST_PORT} + $OFFSET` \ -D hbase.@HBASE_DAEMON@.info.port=`expr ${FIRST_INFO_PORT} + ${OFFSET}`" Also, the binscripts support pointing them to a configuration directory different than default, so you could do something like: su -s /bin/sh hbase -c "$DAEMON_SCRIPT start --conf $DAEMON_HBASE_ETC @HBASE_DAEMON@" and break out configuration for each daemon instance into /etc/hbase/1/, /etc/hbase/2/, /etc/hbase/3/, ... adjusting hbase-env.sh and hbase-site.xml accordingly in each. It's not as nice as having the service start/stop script do it though, port assignments would need be done by hand.
          Hide
          Biju Nair added a comment - - edited

          Thanks Andrew Purtell. It would be good to support option 1 through service stop|start|... so that multiple -env and -site.xml files doesn't have to be created just for changing the JMX port. Also it will help keep consistency on how to bring up multiple RS with and without JMX monitoring.

          But the challenge is that, the daemon script should not take JMX related parameters and values from hbase-env.sh and only from HBASE_MULTI_ARGS if that is defined. That means some additional logic in the hbase daemon script. Thoughts?

          The starting JMX port can be set in /etc/default/hbase and can be incremented with the offset.

          Show
          Biju Nair added a comment - - edited Thanks Andrew Purtell . It would be good to support option 1 through service stop|start|... so that multiple -env and -site.xml files doesn't have to be created just for changing the JMX port. Also it will help keep consistency on how to bring up multiple RS with and without JMX monitoring. But the challenge is that, the daemon script should not take JMX related parameters and values from hbase-env.sh and only from HBASE_MULTI_ARGS if that is defined. That means some additional logic in the hbase daemon script. Thoughts? The starting JMX port can be set in /etc/default/hbase and can be incremented with the offset.
          Hide
          Andrew Purtell added a comment -

          Are you thinking of providing a patch Biju Nair?

          Show
          Andrew Purtell added a comment - Are you thinking of providing a patch Biju Nair ?
          Hide
          Biju Nair added a comment -

          Andrew Purtell, Please refer to BIGTOP-1632 for the patch.

          Show
          Biju Nair added a comment - Andrew Purtell , Please refer to BIGTOP-1632 for the patch.
          Hide
          Biju Nair added a comment -

          Few comments/questions

          • If an offset is not passed for service action (start|stop|...), is there a reason why the service action can't be taken with offset set to 0. This will make sure that the logic is in one place whether action is taken for multiple RS or single RS.
          • Is there a reason why we want users to have the _OFFSETS file under /etc/init.d folder and not under /etc/hbase/conf?
          Show
          Biju Nair added a comment - Few comments/questions If an offset is not passed for service action (start|stop|...), is there a reason why the service action can't be taken with offset set to 0. This will make sure that the logic is in one place whether action is taken for multiple RS or single RS. Is there a reason why we want users to have the _OFFSETS file under /etc/init.d folder and not under /etc/hbase/conf?
          Hide
          Sean Mackrory added a comment -

          Is there a reason why we want users to have the _OFFSETS file under /etc/init.d folder and not under /etc/hbase/conf?

          That comment is actually mistaken, so I'd suggest we correct it in any further patches - I used a separate offsets file in my proof-of-concept, but the final patch actually uses an environment variable which the example suggests should be set in /etc/default/hbase.

          Show
          Sean Mackrory added a comment - Is there a reason why we want users to have the _OFFSETS file under /etc/init.d folder and not under /etc/hbase/conf? That comment is actually mistaken, so I'd suggest we correct it in any further patches - I used a separate offsets file in my proof-of-concept, but the final patch actually uses an environment variable which the example suggests should be set in /etc/default/hbase.
          Hide
          Biju Nair added a comment -

          Thanks Sean Mackrory

          • Any feedback on the first comment. Any reason why we can't set OFFSETS_FROM_CLI to 0 if OFFSETS_FROM_CLI and OFFSETS_FROM_DEFAULT variables are not set and use multi_hbase_daemon "start" to start the region servers. This will help reuse the same code for both single RS and multi RS environment and tighten the code.
          • This will also help reduce logic in other functions like stop() since OFFSETS_FROM_PIDS will be set with values of all running RS PIDS.
          Show
          Biju Nair added a comment - Thanks Sean Mackrory Any feedback on the first comment. Any reason why we can't set OFFSETS_FROM_CLI to 0 if OFFSETS_FROM_CLI and OFFSETS_FROM_DEFAULT variables are not set and use multi_hbase_daemon "start" to start the region servers. This will help reuse the same code for both single RS and multi RS environment and tighten the code . This will also help reduce logic in other functions like stop() since OFFSETS_FROM_PIDS will be set with values of all running RS PIDS.
          Hide
          Sean Mackrory added a comment -

          I think the only drawback to that is that you then end up with a "0" inserted into and .out file and .pid file names, which might be a bit weird for the presumably more common case of just running a single daemon. I agree there's a big benefit to consolidating the logic, though.

          Show
          Sean Mackrory added a comment - I think the only drawback to that is that you then end up with a "0" inserted into and .out file and .pid file names, which might be a bit weird for the presumably more common case of just running a single daemon. I agree there's a big benefit to consolidating the logic, though.
          Hide
          Biju Nair added a comment -

          Agreed Sean Mackrory on the additional "0" in the file .out and .pid file names. But it may be a small price to pay on aesthetics compared to the benefit of consolidated logic that will help with code maintenance and reduce errors.

          Show
          Biju Nair added a comment - Agreed Sean Mackrory on the additional "0" in the file .out and .pid file names. But it may be a small price to pay on aesthetics compared to the benefit of consolidated logic that will help with code maintenance and reduce errors.
          Hide
          Sean Mackrory added a comment -

          Yeah I certainly wouldn't -1 the idea

          Show
          Sean Mackrory added a comment - Yeah I certainly wouldn't -1 the idea
          Hide
          Biju Nair added a comment - - edited

          Andrew Purtell,Roman Shaposhnik and others.. any feedback on the idea to consolidate logic in bringing up single RS and Multi RS.

          Show
          Biju Nair added a comment - - edited Andrew Purtell , Roman Shaposhnik and others.. any feedback on the idea to consolidate logic in bringing up single RS and Multi RS.

            People

            • Assignee:
              Sean Mackrory
              Reporter:
              Sean Mackrory
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development