ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-499

electionAlg should default to FLE (3) - regression

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 3.2.0
    • Fix Version/s: 3.2.1, 3.3.0
    • Component/s: server, tests
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      workaround in 3.2.0 (this only effects 3.2.0)

      set electionAlg=3 in server config files.
      Show
      workaround in 3.2.0 (this only effects 3.2.0) set electionAlg=3 in server config files.

      Description

      there's a regression in 3.2 - electionAlg is no longer defaulting to 3 (incorrectly defaults to 0)

      also - need to have tests to validate this

      1. ZOOKEEPER-499_br3.2.patch
        8 kB
        Patrick Hunt
      2. ZOOKEEPER-499.patch
        10 kB
        Patrick Hunt
      3. ZOOKEEPER-499_br3.2.patch
        7 kB
        Patrick Hunt
      4. ZOOKEEPER-499.patch
        9 kB
        Patrick Hunt

        Activity

        Hide
        Patrick Hunt added a comment -

        patches to fix on trunk and branch (br3.2 is the branch patch)

        this fixes the problem - electionAlg again defaults to 3
        it also adds a test to verify fle is used by default
        it also fixes a test that fails if fle is used (vs algo 0) which is due to a difference in the way jdk exposes
        unresolved host names when using udp vs tcp.

        Show
        Patrick Hunt added a comment - patches to fix on trunk and branch (br3.2 is the branch patch) this fixes the problem - electionAlg again defaults to 3 it also adds a test to verify fle is used by default it also fixes a test that fails if fle is used (vs algo 0) which is due to a difference in the way jdk exposes unresolved host names when using udp vs tcp.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12415671/ZOOKEEPER-499_br3.2.patch
        against trunk revision 801839.

        +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 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-vesta.apache.org/171/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/171/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/171/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/12415671/ZOOKEEPER-499_br3.2.patch against trunk revision 801839. +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 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-vesta.apache.org/171/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/171/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/171/console This message is automatically generated.
        Hide
        Benjamin Reed added a comment -

        this looks good pat, but when you first get the logger, why are you using the package name? if you are going to use the package name shouldn't you get the package from the class file?

        in the second test, you get the logger using a package to add an appender, but remove using the class. couldn't that cause a problem potentially?

        Show
        Benjamin Reed added a comment - this looks good pat, but when you first get the logger, why are you using the package name? if you are going to use the package name shouldn't you get the package from the class file? in the second test, you get the logger using a package to add an appender, but remove using the class. couldn't that cause a problem potentially?
        Hide
        Patrick Hunt added a comment -

        updated patch based on ben's comments:

        1) using the name of the package is fine, this is actually taking advantage of a feature of log4j
        http://logging.apache.org/log4j/1.2/manual.html
        "Each enabled logging request for a given logger will be forwarded to all the appenders in that logger as well as the appenders higher in the hierarchy."

        2) I was removing the appender from the wrong logger, I've updated the patch to correctly remove.

        Show
        Patrick Hunt added a comment - updated patch based on ben's comments: 1) using the name of the package is fine, this is actually taking advantage of a feature of log4j http://logging.apache.org/log4j/1.2/manual.html "Each enabled logging request for a given logger will be forwarded to all the appenders in that logger as well as the appenders higher in the hierarchy." 2) I was removing the appender from the wrong logger, I've updated the patch to correctly remove.
        Hide
        Mahadev konar added a comment -

        +1 this looks good. ..

        Show
        Mahadev konar added a comment - +1 this looks good. ..
        Hide
        Mahadev konar added a comment -

        I just committed this. thanks pat.

        Show
        Mahadev konar added a comment - I just committed this. thanks pat.
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #412 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/412/)
        . electionAlg should default to FLE (3) - regression (phunt via mahadev)

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #412 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/412/ ) . electionAlg should default to FLE (3) - regression (phunt via mahadev)

          People

          • Assignee:
            Patrick Hunt
            Reporter:
            Patrick Hunt
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development