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

        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.
        Hide
        Henry Robinson added a comment -

        Great stuff Chris, thanks very much!

        Show
        Henry Robinson added a comment - Great stuff Chris, thanks very much!
        Hide
        Benjamin Reed added a comment -

        +1 thanks chris!

        Show
        Benjamin Reed added a comment - +1 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
        Mahadev konar added a comment -

        I just committed this. thanks chris.

        Show
        Mahadev konar added a comment - I just committed this. thanks chris.
        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)

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development