ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-482

ignore sigpipe in testRetry to avoid silent immediate failure

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.2.0
    • Fix Version/s: 3.2.1, 3.3.0
    • Component/s: c client, tests
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Ignore SIGPIPE signal to C test suite does not fail suddenly.

      Description

      The testRetry test silently exits for me periodically, especially, it seems, on newer hardware. It also spits out from log messages clutter the test output.

      The silent exits turn out to be because SIGPIPE is sometimes delivered during the sleep(1) in createClient(), the second time createClient() is called. Since SIGPIPE is not being ignored and there is no signal handler, the process exists immediately. This leaves the test suite in a broken state, with the test ZooKeeper process still running because "zkServer.sh stop" is not run by tearDown(). You have to manually kill the ZK server and retry the tests; sometimes they succeed and sometimes they don't.

      I described SIGPIPE handling a little in ZOOKEEPER-320. The appropriate thing, I think, is for the client application to ignore or handle SIGPIPE. In this case, that falls to the test processes. The attached patch fixes the issue for me with testRetry.

      The patch uses sigaction() to ignore SIGPIPE in TestClientRetry.cc and, for good measure (although I never saw it actually fail for me), TestClient.cc, since that file also uses sleep() extensively.

      I also removed a couple of unused functions and a macro definition from TestClientRetry.cc, just to simply matters, and turned off log output, which makes the testRetry output much, much cleaner (otherwise you get a lot of log output spamming into the nice clean cppunit output .

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        2m 12s 1 Chris Darroch 22/Jul/09 22:47
        Patch Available Patch Available Resolved Resolved
        7d 23h 47m 1 Mahadev konar 30/Jul/09 22:34
        Resolved Resolved Closed Closed
        37d 2m 1 Patrick Hunt 05/Sep/09 22:36
        Patrick Hunt made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #400 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/400/)
        . ignore sigpipe in testRetry to avoid silent immediate failure. (chris via mahadev)

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #400 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/400/ ) . ignore sigpipe in testRetry to avoid silent immediate failure. (chris via mahadev)
        Mahadev konar made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Hide
        Mahadev konar added a comment -

        I just committed this. thanks chris.

        Show
        Mahadev konar added a comment - I just committed this. thanks chris.
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 4 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/161/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/161/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/161/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/12414276/ZOOKEEPER-482.patch against trunk revision 798038. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 4 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/161/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/161/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/161/console This message is automatically generated.
        Hide
        Benjamin Reed added a comment -

        +1 thanks chris!

        Show
        Benjamin Reed added a comment - +1 thanks chris!
        Hide
        Henry Robinson added a comment -

        Great stuff Chris, thanks very much!

        Show
        Henry Robinson added a comment - Great stuff Chris, thanks very much!
        Patrick Hunt made changes -
        Assignee Chris Darroch [ cdarroch ]
        Patrick Hunt made changes -
        Fix Version/s 3.3.0 [ 12313976 ]
        Hide
        Mahadev konar added a comment -

        thanks chris,
        I have been meaning to fix the SIGPIPE in the tests but always forgot about it.

        Show
        Mahadev konar added a comment - thanks chris, I have been meaning to fix the SIGPIPE in the tests but always forgot about it.
        Chris Darroch made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Release Note Ignore SIGPIPE signal to C test suite does not fail suddenly.
        Chris Darroch made changes -
        Field Original Value New Value
        Attachment ZOOKEEPER-482.patch [ 12414276 ]
        Chris Darroch created issue -

          People

          • Assignee:
            Chris Darroch
            Reporter:
            Chris Darroch
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development