Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0.0
    • Component/s: hbase
    • Labels:

      Description

      Update made to the HBase init.d script in BIGTOP-732 allows users to bring up more than 1 RS on a node. This task is to make changes to HBase init.d script which will allow users to perform monitoring of all the RS through JMX.

      1. BIGTOP-1632.3.patch
        1 kB
        Biju Nair
      2. BIGTOP-1632-0.patch
        1 kB
        Biju Nair
      3. BIGTOP-1632-1.patch
        2 kB
        Biju Nair
      4. BIGTOP-1632-2.patch
        2 kB
        Biju Nair

        Activity

        Hide
        gsbiju Biju Nair added a comment -

        Patch to set JMX ports for multiple RS running in a single node. The starting JMX port value need to be exported in JMXPORT variable and it can be done in /etc/default/hbase.

        Show
        gsbiju Biju Nair added a comment - Patch to set JMX ports for multiple RS running in a single node. The starting JMX port value need to be exported in JMXPORT variable and it can be done in /etc/default/hbase .
        Hide
        jayunit100 jay vyas added a comment - - edited

        1) Hi Biju Nair , fyi, we sorta prefer for non trivial patches the git format-patch method, which assigns you proper credit... not a huge deal but maybe in the future you might want to use this when submitting patches... https://cwiki.apache.org/confluence/display/BIGTOP/How+to+Contribute

        2) do we need an if/else here? seems like the "else" case actually is just appending one more arg to the end, so it can just be an if, right? i.e. roughly like

        if [ "x$JMXPORT" ! = "x" ] ; then
        HBASE_MULTI_ARGS = "${HBASE_MULTI_ARGS} -Dhbase.@HBASE_DAEMON@.info.port=`expr ${FIRST_INFO_PORT} + ${OFFSET}` -Dcom.sun.management.jmxremote.port=`expr ${JMXPORT} + ${OFFSET}`"
        fi
        

        otherwise, makes sense to me... thanks !

        Show
        jayunit100 jay vyas added a comment - - edited 1) Hi Biju Nair , fyi, we sorta prefer for non trivial patches the git format-patch method, which assigns you proper credit... not a huge deal but maybe in the future you might want to use this when submitting patches... https://cwiki.apache.org/confluence/display/BIGTOP/How+to+Contribute 2) do we need an if/else here? seems like the "else" case actually is just appending one more arg to the end, so it can just be an if , right? i.e. roughly like if [ "x$JMXPORT" ! = "x" ] ; then HBASE_MULTI_ARGS = "${HBASE_MULTI_ARGS} -Dhbase.@HBASE_DAEMON@.info.port=`expr ${FIRST_INFO_PORT} + ${OFFSET}` -Dcom.sun.management.jmxremote.port=`expr ${JMXPORT} + ${OFFSET}`" fi otherwise, makes sense to me... thanks !
        Hide
        gsbiju Biju Nair added a comment -

        jay vyas Thanks for the comments.
        1) Will update the patch following the standard outlined.
        2) Definitely if/else is not required.

        Show
        gsbiju Biju Nair added a comment - jay vyas Thanks for the comments. 1) Will update the patch following the standard outlined. 2) Definitely if/else is not required.
        Hide
        gsbiju Biju Nair added a comment -

        Updated patch for review comments.

        Show
        gsbiju Biju Nair added a comment - Updated patch for review comments.
        Hide
        apurtell Andrew Purtell added a comment -

        Patch #1 is missing the update to /etc/default/hbase ?

        Show
        apurtell Andrew Purtell added a comment - Patch #1 is missing the update to /etc/default/hbase ?
        Hide
        rvs Roman Shaposhnik added a comment -

        Looks reasonable to me, but I'd defer to Andrew Purtell for an actual vote

        Show
        rvs Roman Shaposhnik added a comment - Looks reasonable to me, but I'd defer to Andrew Purtell for an actual vote
        Hide
        gsbiju Biju Nair added a comment -

        Andrew Purtell Changes to /etc/default/hbase to export JMXPORT variable included in the patch.

        Show
        gsbiju Biju Nair added a comment - Andrew Purtell Changes to /etc/default/hbase to export JMXPORT variable included in the patch.
        Hide
        gsbiju Biju Nair added a comment -

        As commented in BIGTOP-732, the logic to bring up a single RS is different from multi RS i.e. users need to set the port value for JMX either in hbase-env.sh or in /etc/default/hbase. That being the reason for leaving the change in /etc/default/hbase. Attached a new patch with the change required in /etc/default/hbase in comments.

        Show
        gsbiju Biju Nair added a comment - As commented in BIGTOP-732 , the logic to bring up a single RS is different from multi RS i.e. users need to set the port value for JMX either in hbase-env.sh or in /etc/default/hbase . That being the reason for leaving the change in /etc/default/hbase . Attached a new patch with the change required in /etc/default/hbase in comments.
        Hide
        apurtell Andrew Purtell added a comment -

        lgtm

        Have you tested bringing up multiple RS with the patch applied? Does it work for you?

        Show
        apurtell Andrew Purtell added a comment - lgtm Have you tested bringing up multiple RS with the patch applied? Does it work for you?
        Hide
        gsbiju Biju Nair added a comment -

        Andrew Purtell.I did test by bringing up multiple RS with and without JMXPORT variable set.

        Show
        gsbiju Biju Nair added a comment - Andrew Purtell .I did test by bringing up multiple RS with and without JMXPORT variable set.
        Hide
        apurtell Andrew Purtell added a comment -

        Ok, committing shortly unless there's an objection.

        Show
        apurtell Andrew Purtell added a comment - Ok, committing shortly unless there's an objection.
        Hide
        apurtell Andrew Purtell added a comment -

        Committed. Thanks for the patch Biju Nair

        Show
        apurtell Andrew Purtell added a comment - Committed. Thanks for the patch Biju Nair
        Hide
        gsbiju Biju Nair added a comment -

        In the earlier patch provided the jmx port attribute -Dcom.sun.management.jmxremote.port was set in a location where it didn't have any effect since it is taken as an value passed to the HBase daemon instead of the JVM. This patch is to fix the issue.

        Note: The earlier patch didn't have any impact on the function to manage HBase except that the new parameter was ignored.

        Show
        gsbiju Biju Nair added a comment - In the earlier patch provided the jmx port attribute -Dcom.sun.management.jmxremote.port was set in a location where it didn't have any effect since it is taken as an value passed to the HBase daemon instead of the JVM. This patch is to fix the issue. Note: The earlier patch didn't have any impact on the function to manage HBase except that the new parameter was ignored.

          People

          • Assignee:
            gsbiju Biju Nair
            Reporter:
            gsbiju Biju Nair
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development