ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1946

Server logging should reflect dynamically reconfigured address

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.5.0
    • Fix Version/s: 3.5.0
    • Component/s: server
    • Labels:
    • Hadoop Flags:
      Reviewed

      Description

      The server's client address:port is part of the QuorumPeer's thread name and thus shown in logs. Thread name is not currently updated after dynamic reconfiguration of the address, resulting in confusing log entries.

      1. ZOOKEEPER-1946.patch
        2 kB
        Niko Vuokko
      2. ZOOKEEPER-1946.patch
        1 kB
        Niko Vuokko
      3. ZOOKEEPER-1946.patch
        1 kB
        Niko Vuokko

        Activity

        Hide
        Raul Gutierrez Segales added a comment -

        Hi Niko Vuokko, are you planning to propose a patch for this? If so I'll assign it to you (I am happy to help with reviews as well).

        Show
        Raul Gutierrez Segales added a comment - Hi Niko Vuokko , are you planning to propose a patch for this? If so I'll assign it to you (I am happy to help with reviews as well).
        Hide
        Niko Vuokko added a comment -

        Just wanted to still check if I could put this change to somewhere nicer

        Show
        Niko Vuokko added a comment - Just wanted to still check if I could put this change to somewhere nicer
        Hide
        Hadoop QA added a comment -

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

        +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 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2146//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12652349/ZOOKEEPER-1946.patch against trunk revision 1601516. +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 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2146//console This message is automatically generated.
        Hide
        Niko Vuokko added a comment -

        Manual test for minor fix:
        1) Configure 3 servers (1,2,3) where two first agree on the configuration, but the configuration on server 3 differs on its own ports.
        2) Start servers 1 and 2 and once quorum is achieved, start 3.
        3) QuorumPeer log messages on server 3 first contain the differing port. After joining the quorum the messages switch to containing the port provided by server 1 and 2 quorum.

        Show
        Niko Vuokko added a comment - Manual test for minor fix: 1) Configure 3 servers (1,2,3) where two first agree on the configuration, but the configuration on server 3 differs on its own ports. 2) Start servers 1 and 2 and once quorum is achieved, start 3. 3) QuorumPeer log messages on server 3 first contain the differing port. After joining the quorum the messages switch to containing the port provided by server 1 and 2 quorum.
        Hide
        Niko Vuokko added a comment -

        Anyone know why it couldn't apply the patch?
        It's created straight from git diff against trunk

        Show
        Niko Vuokko added a comment - Anyone know why it couldn't apply the patch? It's created straight from git diff against trunk
        Hide
        Patrick Hunt added a comment -

        Niko Vuokko It's due to the "a" and "b" prefix that git puts onto it's patches by default. Run git diff with the --no-prefix option, see the example/details here: https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute

        Show
        Patrick Hunt added a comment - Niko Vuokko It's due to the "a" and "b" prefix that git puts onto it's patches by default. Run git diff with the --no-prefix option, see the example/details here: https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute
        Hide
        Patrick Hunt added a comment -

        Patch needs to be updated.

        Show
        Patrick Hunt added a comment - Patch needs to be updated.
        Hide
        Raul Gutierrez Segales added a comment -

        Niko Vuokko: what Patrick Hunt said about using --no-prefix from git. Besides that, lgtm +1 - thanks!

        Show
        Raul Gutierrez Segales added a comment - Niko Vuokko : what Patrick Hunt said about using --no-prefix from git. Besides that, lgtm +1 - thanks!
        Hide
        Niko Vuokko added a comment -

        Fixed patch formatting

        Show
        Niko Vuokko added a comment - Fixed patch formatting
        Hide
        Alexander Shraer added a comment -

        Hi Nikko,

        The patch looks good, thanks! my only suggestion is to modify testPortChange() in src/java/test/org/apache/zookeeper/test/ReconfigTest.java to check for the thread id before and after the reconfig.

        Thanks,
        Alex

        Show
        Alexander Shraer added a comment - Hi Nikko, The patch looks good, thanks! my only suggestion is to modify testPortChange() in src/java/test/org/apache/zookeeper/test/ReconfigTest.java to check for the thread id before and after the reconfig. Thanks, Alex
        Hide
        Niko Vuokko added a comment -

        Test added. I don't think checking the thread name prior to reconfig is necessary since the other more relevant tests aren't that paranoid either.

        Show
        Niko Vuokko added a comment - Test added. I don't think checking the thread name prior to reconfig is necessary since the other more relevant tests aren't that paranoid either.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12652591/ZOOKEEPER-1946.patch
        against trunk revision 1605517.

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

        +1 tests included. The patch appears to include 3 new or modified tests.

        +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/2155//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2155//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2155//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12652591/ZOOKEEPER-1946.patch against trunk revision 1605517. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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/2155//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2155//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2155//console This message is automatically generated.
        Hide
        Alexander Shraer added a comment -

        +1. Thanks Niko!

        Show
        Alexander Shraer added a comment - +1. Thanks Niko!
        Hide
        Patrick Hunt added a comment -

        Commtited to trunk. Thanks Niko!

        Show
        Patrick Hunt added a comment - Commtited to trunk. Thanks Niko!
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in ZooKeeper-trunk #2354 (See https://builds.apache.org/job/ZooKeeper-trunk/2354/)
        ZOOKEEPER-1946. Server logging should reflect dynamically reconfigured address (Niko Vuokko via phunt and Alexander Shraer) (phunt: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1607525)

        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
        • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ReconfigTest.java
        Show
        Hudson added a comment - SUCCESS: Integrated in ZooKeeper-trunk #2354 (See https://builds.apache.org/job/ZooKeeper-trunk/2354/ ) ZOOKEEPER-1946 . Server logging should reflect dynamically reconfigured address (Niko Vuokko via phunt and Alexander Shraer) (phunt: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1607525 ) /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ReconfigTest.java

          People

          • Assignee:
            Niko Vuokko
            Reporter:
            Niko Vuokko
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development