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

        Niko Vuokko created issue -
        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).
        Niko Vuokko made changes -
        Field Original Value New Value
        Status Open [ 1 ] Patch Available [ 10002 ]
        Niko Vuokko made changes -
        Attachment ZOOKEEPER-1946.patch [ 12652349 ]
        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
        Patrick Hunt made changes -
        Assignee Niko Vuokko [ vuakko ]
        Hide
        Patrick Hunt added a comment -

        Patch needs to be updated.

        Show
        Patrick Hunt added a comment - Patch needs to be updated.
        Patrick Hunt made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        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
        Niko Vuokko made changes -
        Attachment ZOOKEEPER-1946.patch [ 12652490 ]
        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.
        Niko Vuokko made changes -
        Attachment ZOOKEEPER-1946.patch [ 12652591 ]
        Niko Vuokko made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        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!
        Patrick Hunt made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Fix Version/s 3.5.0 [ 12316644 ]
        Resolution Fixed [ 1 ]
        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
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        10h 18m 1 Patrick Hunt 25/Jun/14 18:20
        Open Open Patch Available Patch Available
        1d 4h 25m 2 Niko Vuokko 26/Jun/14 21:18
        Patch Available Patch Available Resolved Resolved
        6d 7h 44m 1 Patrick Hunt 03/Jul/14 05:02

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development