Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3.1
    • Fix Version/s: 3.3.2, 3.4.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Notification messages are quite important to determine what is going with leader election. The main idea of this improvement is name the fields we output in notification log messages.

      1. ZOOKEEPER-789.patch
        3 kB
        Flavio Junqueira
      2. ZOOKEEPER-789.patch
        3 kB
        Flavio Junqueira
      3. ZOOKEEPER-789.patch
        3 kB
        Flavio Junqueira
      4. ZOOKEEPER-789.patch
        36 kB
        Patrick Hunt
      There are no Sub-Tasks for this issue.

        Activity

        Hide
        Flavio Junqueira added a comment -

        I have created a method to print info about a notification and placed the call to the method to right after the notification structure is created in FastLeaderElection.WorkerReceiver.run(). This way all received notifications are logged.

        There is no test, since this is just a modification to message logging.

        Show
        Flavio Junqueira added a comment - I have created a method to print info about a notification and placed the call to the method to right after the notification structure is created in FastLeaderElection.WorkerReceiver.run(). This way all received notifications are logged. There is no test, since this is just a modification to message logging.
        Hide
        Hadoop QA added a comment -

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

        +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 tests are needed for 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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/119/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/119/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/119/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/12447713/ZOOKEEPER-789.patch against trunk revision 953041. +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 tests are needed for 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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/119/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/119/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/119/console This message is automatically generated.
        Hide
        Patrick Hunt added a comment -

        Flavio can you resubmit with using tabs for indentation? (good to update your editor to only use spaces, eclipse?) Thanks.

        Show
        Patrick Hunt added a comment - Flavio can you resubmit with using tabs for indentation? (good to update your editor to only use spaces, eclipse?) Thanks.
        Hide
        Flavio Junqueira added a comment -

        Thanks for pointing it out, Pat. I forgot to edit the preferences (new desktop). Here is a new patch.

        Show
        Flavio Junqueira added a comment - Thanks for pointing it out, Pat. I forgot to edit the preferences (new desktop). Here is a new patch.
        Hide
        Ivan Kelly added a comment -

        The patch attached passes each member of Notification separatedly to printNotification. Would it not be simpler to just pass the whole Notification object to printNotification. This way if the Notification object changes internally, only printNotification needs to be changed.

        Show
        Ivan Kelly added a comment - The patch attached passes each member of Notification separatedly to printNotification. Would it not be simpler to just pass the whole Notification object to printNotification. This way if the Notification object changes internally, only printNotification needs to be changed.
        Hide
        Ivan Kelly added a comment -

        Fixed up loggraph parsing, and changed printNotification to take the Notification object itself rather than all the little parts.

        Show
        Ivan Kelly added a comment - Fixed up loggraph parsing, and changed printNotification to take the Notification object itself rather than all the little parts.
        Hide
        Flavio Junqueira added a comment -

        It is a good suggestion. Uploading a new patch...

        Show
        Flavio Junqueira added a comment - It is a good suggestion. Uploading a new patch...
        Hide
        Hadoop QA added a comment -

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

        +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 tests are needed for 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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/126/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/126/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/126/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/12448310/ZOOKEEPER-789.patch against trunk revision 958096. +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 tests are needed for 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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/126/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/126/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/126/console This message is automatically generated.
        Hide
        Flavio Junqueira added a comment -

        There is no need for testing here. This is just a modification to log messages.

        Show
        Flavio Junqueira added a comment - There is no need for testing here. This is just a modification to log messages.
        Hide
        Hadoop QA added a comment -

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

        +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 tests are needed for 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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/128/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/128/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/128/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/12448310/ZOOKEEPER-789.patch against trunk revision 958096. +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 tests are needed for 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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/128/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/128/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/128/console This message is automatically generated.
        Hide
        Patrick Hunt added a comment -

        The patch still had tab issues. I cleaned it up using emacs to untabify the entire file (including the original patch changes, the cleanup tool cleans up both tabs and unnecessary whitespace issues). so this latest patch has only whitespace changes, not functional differences.

        Show
        Patrick Hunt added a comment - The patch still had tab issues. I cleaned it up using emacs to untabify the entire file (including the original patch changes, the cleanup tool cleans up both tabs and unnecessary whitespace issues). so this latest patch has only whitespace changes, not functional differences.
        Hide
        Patrick Hunt added a comment -

        +1, thanks flavio.

        Show
        Patrick Hunt added a comment - +1, thanks flavio.
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #869 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/869/)
        ZOOKEEPER-789. Improve FLE log messages

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #869 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/869/ ) ZOOKEEPER-789 . Improve FLE log messages

          People

          • Assignee:
            Flavio Junqueira
            Reporter:
            Flavio Junqueira
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development