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. warnings.txt
        10 kB
        Michi Mutsuzaki
      2. ZOOKEEPER-864.diff
        169 kB
        Ivan Kelly
      3. ZOOKEEPER-864.diff
        147 kB
        Ivan Kelly
      4. ZOOKEEPER-864.diff
        145 kB
        Ivan Kelly
      5. ZOOKEEPER-864.diff
        143 kB
        Ivan Kelly

        Activity

        Ivan Kelly created issue -
        Ivan Kelly made changes -
        Field Original Value New Value
        Attachment ZOOKEEPER-864.diff [ 12453779 ]
        Ivan Kelly made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        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
        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 -

        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
        Michi Mutsuzaki made changes -
        Attachment warnings.txt [ 12453863 ]
        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
        Ivan Kelly made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Ivan Kelly made changes -
        Attachment ZOOKEEPER-864.diff [ 12453950 ]
        Ivan Kelly made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        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 -
        Attachment ZOOKEEPER-864.diff [ 12454033 ]
        Hide
        Ivan Kelly added a comment -

        cancelling

        Show
        Ivan Kelly added a comment - cancelling
        Ivan Kelly made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        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 Open [ 1 ] Patch Available [ 10002 ]
        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
        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
        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 -

        +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
        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.
        Ivan Kelly made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Ivan Kelly made changes -
        Attachment ZOOKEEPER-864.diff [ 12455251 ]
        Ivan Kelly made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Benjamin Reed added a comment -

        thanx ivan!
        Committed revision 1021463.

        Show
        Benjamin Reed added a comment - thanx ivan! Committed revision 1021463.
        Benjamin Reed made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        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
        Mahadev konar made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development