Uploaded image for project: 'ZooKeeper'
  1. ZooKeeper
  2. ZOOKEEPER-1625

zkServer.sh is looking for clientPort in config file, but it may no longer be there with ZK-1411

    Details

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

      Description

      zkServer.sh is currently looking for "clientPort" entry in the static configuration file and uses it to contact the server.

      With ZOOKEEPER-1411 clientPort is part of the dynamic configuration, and may appear in the separate dynamic configuration file. The "clientPort" entry may no longer be in the static config file.

      With the proposed patch zkServer.sh first looks in the old (static) config file, then if clientPort is not there, it figures out the id of the server by looking at myid file, and then using that id finds the client port in the dynamic config file.

      1. ZOOKEEPER-1625.patch
        1 kB
        Alexander Shraer

        Issue Links

          Activity

          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12565908/ZOOKEEPER-1625.patch
          against trunk revision 1434978.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1352//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1352//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1352//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12565908/ZOOKEEPER-1625.patch against trunk revision 1434978. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1352//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1352//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1352//console This message is automatically generated.
          Hide
          shralex Alexander Shraer added a comment -

          The patch includes only changes to zkServer.sh. I manually verified that it finds the client port in static or dynamic config file or reports an error if the port is not found.

          Show
          shralex Alexander Shraer added a comment - The patch includes only changes to zkServer.sh. I manually verified that it finds the client port in static or dynamic config file or reports an error if the port is not found.
          Hide
          michim Michi Mutsuzaki added a comment -

          +1

          --Michi

          Show
          michim Michi Mutsuzaki added a comment - +1 --Michi
          Hide
          hudson Hudson added a comment -

          Integrated in ZooKeeper-trunk #1811 (See https://builds.apache.org/job/ZooKeeper-trunk/1811/)
          ZOOKEEPER-1625. zkServer.sh is looking for clientPort in config file, but it may
          no longer be there with ZK-1411 (Alexander Shraer via michim) (Revision 1437257)

          Result = SUCCESS
          michim : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1437257
          Files :

          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/bin/zkServer.sh
          Show
          hudson Hudson added a comment - Integrated in ZooKeeper-trunk #1811 (See https://builds.apache.org/job/ZooKeeper-trunk/1811/ ) ZOOKEEPER-1625 . zkServer.sh is looking for clientPort in config file, but it may no longer be there with ZK-1411 (Alexander Shraer via michim) (Revision 1437257) Result = SUCCESS michim : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1437257 Files : /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/bin/zkServer.sh
          Hide
          phunt Patrick Hunt added a comment -

          Previously we relied on the server code itself to check that the client port parameter was set, however now we're additionally checking in the zkServer script. Seems like an unnecessary check.

          also is the following necessary/useful?

          + echo "Client port not found in static config file. Looking in dynamic config file."

          Show
          phunt Patrick Hunt added a comment - Previously we relied on the server code itself to check that the client port parameter was set, however now we're additionally checking in the zkServer script. Seems like an unnecessary check. also is the following necessary/useful? + echo "Client port not found in static config file. Looking in dynamic config file."
          Hide
          shralex Alexander Shraer added a comment -

          Hi Patrick,

          with 1411 the client port may be correctly defined in the dynamic configuration file - it doesn't need to be in the old config file, the server will not detect that anything is wrong because everything is actually fine. The message you would get when running "zkServer.sh status" is "the server is probably not running", which is not helpful - the server is running but the script didn't find the clientport. This should be reported differently by the script. The message you quote is indeed not necessary, but may be useful to identify what really happened if something goes wrong.

          Show
          shralex Alexander Shraer added a comment - Hi Patrick, with 1411 the client port may be correctly defined in the dynamic configuration file - it doesn't need to be in the old config file, the server will not detect that anything is wrong because everything is actually fine. The message you would get when running "zkServer.sh status" is "the server is probably not running", which is not helpful - the server is running but the script didn't find the clientport. This should be reported differently by the script. The message you quote is indeed not necessary, but may be useful to identify what really happened if something goes wrong.

            People

            • Assignee:
              shralex Alexander Shraer
              Reporter:
              shralex Alexander Shraer
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development