Uploaded image for project: 'Bigtop'
  1. Bigtop
  2. BIGTOP-1048

ZooKeeper installation use "zookeeper-server.pid" as default while ZooKeeper expects zookeeper_server.pid

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.5.0
    • Fix Version/s: 0.8.0
    • Component/s: Init scripts
    • Labels:
      None

      Description

      In ZooKeeper file ./bin/zkServer.sh, it uses 'zookeeper_server.pid' as default pid file name like the following:

      ZOOPIDFILE="$ZOO_DATADIR/zookeeper_server.pid"
      
      1. bigtop-1048.patch
        5 kB
        Jeffrey Zhong

        Activity

        Hide
        mackrorysd Sean Mackrory added a comment -

        The change seems safe enough and consistency would be good, but for the sake of testing, could you elaborate on whether or not this caused any functional problems? As far as I can see, ZooKeeper's scripts also expect the PID file to be in the data directory, which we set to /var/lib/zookeeper (whereas currently the PID file is stored under /var/run/zookeeper), so I'm curious to know how this patch changes the behavior of ZooKeeper.

        Show
        mackrorysd Sean Mackrory added a comment - The change seems safe enough and consistency would be good, but for the sake of testing, could you elaborate on whether or not this caused any functional problems? As far as I can see, ZooKeeper's scripts also expect the PID file to be in the data directory, which we set to /var/lib/zookeeper (whereas currently the PID file is stored under /var/run/zookeeper), so I'm curious to know how this patch changes the behavior of ZooKeeper.
        Hide
        jeffreyz Jeffrey Zhong added a comment -

        The patch is mainly for consistency. If ZOO_DATADIR is different from /var/run/zookeeper then I think ZOOPIDFILE has to be set correctly in order for zkServer.sh to work correctly.

        Show
        jeffreyz Jeffrey Zhong added a comment - The patch is mainly for consistency. If ZOO_DATADIR is different from /var/run/zookeeper then I think ZOOPIDFILE has to be set correctly in order for zkServer.sh to work correctly.
        Hide
        cos Konstantin Boudnik added a comment -

        Sean Mackrory, if the patch is good to go - shall we commit and close the ticket? It was open for almost a year now.

        Show
        cos Konstantin Boudnik added a comment - Sean Mackrory , if the patch is good to go - shall we commit and close the ticket? It was open for almost a year now.
        Hide
        mackrorysd Sean Mackrory added a comment -

        Well I don't see a reason for this patch. ZooKeeper currently works because we're setting ZOOPIDFILE to what the init scripts expect it to be. zkServer.sh will set a default pid file location if you don't, but it does it in a location we don't want the pid file to be, which is probably why we're overriding it. I don't see a reason to make it more similar to the default location unless our override isn't working completely. As far as I can tell, it is.

        Show
        mackrorysd Sean Mackrory added a comment - Well I don't see a reason for this patch. ZooKeeper currently works because we're setting ZOOPIDFILE to what the init scripts expect it to be. zkServer.sh will set a default pid file location if you don't, but it does it in a location we don't want the pid file to be, which is probably why we're overriding it. I don't see a reason to make it more similar to the default location unless our override isn't working completely. As far as I can tell, it is.
        Hide
        jeffreyz Jeffrey Zhong added a comment -

        This will cause confusion. For example, if I'm a zookeeper people, I'd search file "zookeeper_server.pid" in zookeeper deployment while bigtop uses a different name. I can see you want to use different folder to put pid file but why using a different name which itself is very similar to zookeeper default pid file name. Only difference is "-" from "_" which is more like a typo to me.

        Show
        jeffreyz Jeffrey Zhong added a comment - This will cause confusion. For example, if I'm a zookeeper people, I'd search file "zookeeper_server.pid" in zookeeper deployment while bigtop uses a different name. I can see you want to use different folder to put pid file but why using a different name which itself is very similar to zookeeper default pid file name. Only difference is "-" from "_" which is more like a typo to me.
        Hide
        mackrorysd Sean Mackrory added a comment -

        Ok. +1. Would you like to submit your patch as the output of "git format-patch"? That way the commit can have your information attached to it. Otherwise I'm happy to just commit it as-is and add "contibuted by Jeffery Zhong" to the commit message.

        Show
        mackrorysd Sean Mackrory added a comment - Ok. +1. Would you like to submit your patch as the output of "git format-patch"? That way the commit can have your information attached to it. Otherwise I'm happy to just commit it as-is and add "contibuted by Jeffery Zhong" to the commit message.
        Hide
        jeffreyz Jeffrey Zhong added a comment -

        Thanks Sean Mackrory for reconsidering! Please commit it as-is. Thanks again.

        Show
        jeffreyz Jeffrey Zhong added a comment - Thanks Sean Mackrory for reconsidering! Please commit it as-is. Thanks again.
        Hide
        mackrorysd Sean Mackrory added a comment -

        Committed!

        Show
        mackrorysd Sean Mackrory added a comment - Committed!
        Hide
        cos Konstantin Boudnik added a comment -

        Thanks Jeffrey! I also have added you to the list of the project's contributors and assigned the ticket to you. Welcome!

        Show
        cos Konstantin Boudnik added a comment - Thanks Jeffrey! I also have added you to the list of the project's contributors and assigned the ticket to you. Welcome!
        Hide
        mackrorysd Sean Mackrory added a comment -

        Konstantin Boudnik - thanks for doing that. I tried to, but could get his name to come up. How do you add someone to the project's contributors? Or does one have to be Cos before they can do that?

        Show
        mackrorysd Sean Mackrory added a comment - Konstantin Boudnik - thanks for doing that. I tried to, but could get his name to come up. How do you add someone to the project's contributors? Or does one have to be Cos before they can do that?
        Hide
        cos Konstantin Boudnik added a comment -

        you just need to have JIRA project admin permissions.

        Show
        cos Konstantin Boudnik added a comment - you just need to have JIRA project admin permissions.

          People

          • Assignee:
            jeffreyz Jeffrey Zhong
            Reporter:
            jeffreyz Jeffrey Zhong
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development