Details

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

      Description

      I changed the socket code to use boost asio. Now the client only creates one thread, and all operations are non-blocking.

      Tests are now automated, just run "make check".

      1. ZOOKEEPER-864.diff
        143 kB
        Ivan Kelly
      2. ZOOKEEPER-864.diff
        145 kB
        Ivan Kelly
      3. ZOOKEEPER-864.diff
        147 kB
        Ivan Kelly
      4. ZOOKEEPER-864.diff
        169 kB
        Ivan Kelly
      5. warnings.txt
        10 kB
        Michi Mutsuzaki

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        18d 21h 7m 3 Ivan Kelly 22/Sep/10 11:54
        Open Open Patch Available Patch Available
        5m 55s 4 Ivan Kelly 22/Sep/10 11:56
        Patch Available Patch Available Resolved Resolved
        19d 7h 4m 1 Benjamin Reed 11/Oct/10 19:01
        Resolved Resolved Closed Closed
        408d 21m 1 Mahadev konar 23/Nov/11 19:22
        Mahadev konar made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #964 (See https://hudson.apache.org/hudson/job/ZooKeeper-trunk/964/)
        ZOOKEEPER-864. Hedwig C++ client improvements

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #964 (See https://hudson.apache.org/hudson/job/ZooKeeper-trunk/964/ ) ZOOKEEPER-864 . Hedwig C++ client improvements
        Benjamin Reed made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Hide
        Benjamin Reed added a comment -

        thanx ivan!
        Committed revision 1021463.

        Show
        Benjamin Reed added a comment - thanx ivan! Committed revision 1021463.
        Ivan Kelly made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Ivan Kelly made changes -
        Attachment ZOOKEEPER-864.diff [ 12455251 ]
        Ivan Kelly made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12454033/ZOOKEEPER-864.diff
        against trunk revision 998200.

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

        +1 tests included. The patch appears to include 34 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 generated 27 release audit warnings (more than the trunk's current 26 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-h7.grid.sp2.yahoo.net/117/testReport/
        Release audit warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/117/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/117/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/117/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/12454033/ZOOKEEPER-864.diff against trunk revision 998200. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 34 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 generated 27 release audit warnings (more than the trunk's current 26 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-h7.grid.sp2.yahoo.net/117/testReport/ Release audit warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/117/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/117/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/117/console This message is automatically generated.
        Hide
        Erwin Tam added a comment -

        +1
        Latest patch looks good to me. We can just approve it and push it in first. Thanks for the great work Ivan!

        Show
        Erwin Tam added a comment - +1 Latest patch looks good to me. We can just approve it and push it in first. Thanks for the great work Ivan!
        Hide
        Ivan Kelly added a comment -

        Yes, you are absolutely right. Unless we don't "install" protocol.h at all and try to remove it from the hedwig public headers. I remember the protocol.h stuff was causing you problems recently so this could resolve that issue too.

        Show
        Ivan Kelly added a comment - Yes, you are absolutely right. Unless we don't "install" protocol.h at all and try to remove it from the hedwig public headers. I remember the protocol.h stuff was causing you problems recently so this could resolve that issue too.
        Hide
        Erwin Tam added a comment -

        Ivan, the changes look good to me. One question though, do we need to include the auto-generated protocol.h header to the Makefile.am? I had to do that step manually a couple of weeks ago otherwise things didn't work when using the hedwig client libs generated.

        Show
        Erwin Tam added a comment - Ivan, the changes look good to me. One question though, do we need to include the auto-generated protocol.h header to the Makefile.am? I had to do that step manually a couple of weeks ago otherwise things didn't work when using the hedwig client libs generated.
        Hide
        Michi Mutsuzaki added a comment -

        +1.

        The latest patch compiles cleanly with no warnings. Cool!

        I'm using gcc 3.4.6.

        $ /usr/bin/g++ --version
        g++ (GCC) 3.4.6 20060404 (Red Hat 3.4.6-9)

        --Michi

        Show
        Michi Mutsuzaki added a comment - +1. The latest patch compiles cleanly with no warnings. Cool! I'm using gcc 3.4.6. $ /usr/bin/g++ --version g++ (GCC) 3.4.6 20060404 (Red Hat 3.4.6-9) --Michi
        Ivan Kelly made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Ivan Kelly added a comment -

        Made the configuration code more generic.

        Show
        Ivan Kelly added a comment - Made the configuration code more generic.
        Ivan Kelly made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Ivan Kelly added a comment -

        cancelling

        Show
        Ivan Kelly added a comment - cancelling
        Ivan Kelly made changes -
        Attachment ZOOKEEPER-864.diff [ 12454033 ]
        Hide
        Ivan Kelly added a comment -

        Fixed all warnings. New patch attached.

        Show
        Ivan Kelly added a comment - Fixed all warnings. New patch attached.
        Ivan Kelly made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Ivan Kelly made changes -
        Attachment ZOOKEEPER-864.diff [ 12453950 ]
        Ivan Kelly made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Ivan Kelly added a comment -

        Ah, these are all trivial to fix. I wonder why none of the compilers I used flagged them though. What version of gcc are you using?

        -Ivan

        Show
        Ivan Kelly added a comment - Ah, these are all trivial to fix. I wonder why none of the compilers I used flagged them though. What version of gcc are you using? -Ivan
        Michi Mutsuzaki made changes -
        Attachment warnings.txt [ 12453863 ]
        Hide
        Michi Mutsuzaki added a comment -

        Here are my comments:

        • After applying the patch, I was able to compile cpp client.
        • I still see some compilation warnings. I'll attach the output.

        Thanks!
        --Michi

        Show
        Michi Mutsuzaki added a comment - Here are my comments: After applying the patch, I was able to compile cpp client. I still see some compilation warnings. I'll attach the output. Thanks! --Michi
        Hide
        Mahadev konar added a comment -

        michi to answer your question,
        all we need is a careful review.

        Show
        Mahadev konar added a comment - michi to answer your question, all we need is a careful review.
        Hide
        Michi Mutsuzaki added a comment -

        Thanks for the patch, Ivan!

        What do we need to do before we can check in this patch?

        --Michi

        Show
        Michi Mutsuzaki added a comment - Thanks for the patch, Ivan! What do we need to do before we can check in this patch? --Michi
        Ivan Kelly made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Ivan Kelly made changes -
        Field Original Value New Value
        Attachment ZOOKEEPER-864.diff [ 12453779 ]
        Ivan Kelly created issue -

          People

          • Assignee:
            Ivan Kelly
            Reporter:
            Ivan Kelly
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development