ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-992

MT Native Version of Windows C Client

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.4.0
    • Component/s: c client
    • Labels:
      None
    • Environment:

      Windows 32

      Description

      This is an extention of the work in https://issues.apache.org/jira/browse/ZOOKEEPER-859

      1. win32_patch.txt
        112 kB
        Dheeraj Agrawal
      2. win32_patch_notabs.txt
        120 kB
        Dheeraj Agrawal
      3. errors.txt
        49 kB
        Michi Mutsuzaki
      4. win32-odysseus-vc2k5.patch
        76 kB
        Erik Anderson
      5. ZOOKEEPER-992.patch
        87 kB
        Dheeraj Agrawal
      6. ZOOKEEPER-992-final.patch
        86 kB
        Dheeraj Agrawal
      7. ZOOKEEPER_992_final.patch
        86 kB
        Dheeraj Agrawal
      8. ZOOKEEPER_992_FINAL.patch
        86 kB
        Dheeraj Agrawal
      9. ZOOKEEPER-992-final.patch
        86 kB
        Mahadev konar
      10. ZOOKEEPER-992-round8.patch
        77 kB
        Erik Anderson
      11. ZOOKEEPER-992.patch
        82 kB
        Dheeraj Agrawal

        Activity

        Camille Fournier created issue -
        Hide
        Dheeraj Agrawal added a comment -

        This patch has changes to make C bindings portable on windows. Its compiled using VisualStudio and the patch has the proj files.
        It support both single and multithreaded versions.

        Show
        Dheeraj Agrawal added a comment - This patch has changes to make C bindings portable on windows. Its compiled using VisualStudio and the patch has the proj files. It support both single and multithreaded versions.
        Dheeraj Agrawal made changes -
        Field Original Value New Value
        Attachment win32_patch.txt [ 12471308 ]
        Camille Fournier made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12471308/win32_patch.txt
        against trunk revision 1069169.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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 (version 1.3.9) warnings.

        -1 release audit. The applied patch generated 27 release audit warnings (more than the trunk's current 24 warnings).

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/136//testReport/
        Release audit warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/136//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/136//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/136//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/12471308/win32_patch.txt against trunk revision 1069169. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 (version 1.3.9) warnings. -1 release audit. The applied patch generated 27 release audit warnings (more than the trunk's current 24 warnings). +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/136//testReport/ Release audit warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/136//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/136//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/136//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12471308/win32_patch.txt
        against trunk revision 1072085.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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 (version 1.3.9) warnings.

        -1 release audit. The applied patch generated 27 release audit warnings (more than the trunk's current 24 warnings).

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/141//testReport/
        Release audit warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/141//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/141//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/141//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/12471308/win32_patch.txt against trunk revision 1072085. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 (version 1.3.9) warnings. -1 release audit. The applied patch generated 27 release audit warnings (more than the trunk's current 24 warnings). -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/141//testReport/ Release audit warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/141//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/141//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/141//console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12471308/win32_patch.txt
        against trunk revision 1072085.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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 (version 1.3.9) warnings.

        -1 release audit. The applied patch generated 27 release audit warnings (more than the trunk's current 24 warnings).

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/160//testReport/
        Release audit warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/160//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/160//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/160//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/12471308/win32_patch.txt against trunk revision 1072085. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 (version 1.3.9) warnings. -1 release audit. The applied patch generated 27 release audit warnings (more than the trunk's current 24 warnings). -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/160//testReport/ Release audit warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/160//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/160//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/160//console This message is automatically generated.
        Patrick Hunt made changes -
        Assignee Dheeraj Agrawal [ dheerajagrawal ]
        Hide
        Dheeraj Agrawal added a comment -

        Main change in zookeeper.jute.c is around the use of gnu ternary operator (http://en.wikipedia.org/wiki/Operator%3F:#C). The usage rc = rc ? : out->end_record(out, tag); is not compatible in Windows so that changes in zookeeper.jute.c are around making them compatible.
        rc = rc ? rc : out->end_record(out, tag);

        Show
        Dheeraj Agrawal added a comment - Main change in zookeeper.jute.c is around the use of gnu ternary operator ( http://en.wikipedia.org/wiki/Operator%3F:#C ). The usage rc = rc ? : out->end_record(out, tag); is not compatible in Windows so that changes in zookeeper.jute.c are around making them compatible. rc = rc ? rc : out->end_record(out, tag);
        Hide
        Mahadev konar added a comment -

        dheeraj, it would be great to include this. Since this changes the c code a lot, adding more tests and getting folks to confirm that it works on RHEL5/RHEL6, Debian/ Ubuntu/ FreeBSD would be great.

        I can take up rhel 5 and rhel 6.

        Show
        Mahadev konar added a comment - dheeraj, it would be great to include this. Since this changes the c code a lot, adding more tests and getting folks to confirm that it works on RHEL5/RHEL6, Debian/ Ubuntu/ FreeBSD would be great. I can take up rhel 5 and rhel 6.
        Hide
        Dheeraj Agrawal added a comment -

        I have run the existing test suite on RHEL3 and RHEL4

        Show
        Dheeraj Agrawal added a comment - I have run the existing test suite on RHEL3 and RHEL4
        Hide
        Mahadev konar added a comment -

        nice! Can we add some tests with multiple zk servers and see on killing a single zk server we can reconnect to the other one? Or the watches are reset? Since this is a big change, getting some load testing with c clients on a set of 3 servers would be great as well. The unit tests arent really exhaustive. What do you think?

        Show
        Mahadev konar added a comment - nice! Can we add some tests with multiple zk servers and see on killing a single zk server we can reconnect to the other one? Or the watches are reset? Since this is a big change, getting some load testing with c clients on a set of 3 servers would be great as well. The unit tests arent really exhaustive. What do you think?
        Hide
        Michi Mutsuzaki (Inactive) added a comment -

        Hi Dheeraj,

        • The patch contains tab characters. Could you convert them to spaces?
        • It looks like modifying src/c/src/hashtable/hashtable_private.h is not necessary.

        I ran unit test on 32-bit rhel and it passed.

        --Michi

        Show
        Michi Mutsuzaki (Inactive) added a comment - Hi Dheeraj, The patch contains tab characters. Could you convert them to spaces? It looks like modifying src/c/src/hashtable/hashtable_private.h is not necessary. I ran unit test on 32-bit rhel and it passed. --Michi
        Hide
        Dheeraj Agrawal added a comment -

        Hi Michi, I will update the patch with spaces instead of tabs. And yes, hashtable_private.h changes are not needed.

        Show
        Dheeraj Agrawal added a comment - Hi Michi, I will update the patch with spaces instead of tabs. And yes, hashtable_private.h changes are not needed.
        Hide
        Michi Mutsuzaki added a comment -

        Hi Dheeraj,

        How do I compile this on a Windows machine? Do I need Visual Studio?

        Thanks!
        --Michi

        Show
        Michi Mutsuzaki added a comment - Hi Dheeraj, How do I compile this on a Windows machine? Do I need Visual Studio? Thanks! --Michi
        Hide
        Dheeraj Agrawal added a comment -

        Attaching file without tabs

        Show
        Dheeraj Agrawal added a comment - Attaching file without tabs
        Dheeraj Agrawal made changes -
        Attachment win32_patch_notabs.txt [ 12476795 ]
        Hide
        Dheeraj Agrawal added a comment -

        Hi Michi, yes you can compile this with Visual Studio. The patch has zookeeper.vcproj project file

        Show
        Dheeraj Agrawal added a comment - Hi Michi, yes you can compile this with Visual Studio. The patch has zookeeper.vcproj project file
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12476795/win32_patch_notabs.txt
        against trunk revision 1091841.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/227//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/12476795/win32_patch_notabs.txt against trunk revision 1091841. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/227//console This message is automatically generated.
        Hide
        Michi Mutsuzaki added a comment -

        Hi Dheeraj,

        Sorry I haven't had a chance to test this patch. I finally got access to a Windows box, so I should be able to test it some time this week.

        Thanks!
        --Michi

        Show
        Michi Mutsuzaki added a comment - Hi Dheeraj, Sorry I haven't had a chance to test this patch. I finally got access to a Windows box, so I should be able to test it some time this week. Thanks! --Michi
        Michi Mutsuzaki made changes -
        Attachment errors.txt [ 12479240 ]
        Hide
        Michi Mutsuzaki added a comment -

        Hmm, I'm having trouble compiling (sorry I'm not familiar with Windows environment). I'll post the errors I'm getting as an attachment.

        Thanks!
        --Michi

        Show
        Michi Mutsuzaki added a comment - Hmm, I'm having trouble compiling (sorry I'm not familiar with Windows environment). I'll post the errors I'm getting as an attachment. Thanks! --Michi
        Hide
        Mahadev konar added a comment - - edited

        Cancelling patch for now. Dheeraj, would it be possible to update the README in src/c to add instructions on how to compile and use the library in the native windows platform? That would be really helpful with this patch.

        Show
        Mahadev konar added a comment - - edited Cancelling patch for now. Dheeraj, would it be possible to update the README in src/c to add instructions on how to compile and use the library in the native windows platform? That would be really helpful with this patch.
        Mahadev konar made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Erik Anderson added a comment -

        Random comment from a passerby, I recently downloaded this patch (a lot easier than trying to do my own port), and while there were a fair number of additional changes that were required, I appear to have been able to compile this on VS2005.

        I know that this doesn't mean anything without a patch (and I suspect it would take a couple hours for me to do the necessary SVN checkouts and diff-minimization), but I would like to avoid seeing the overall bug here being cancelled.

        Show
        Erik Anderson added a comment - Random comment from a passerby, I recently downloaded this patch (a lot easier than trying to do my own port), and while there were a fair number of additional changes that were required, I appear to have been able to compile this on VS2005. I know that this doesn't mean anything without a patch (and I suspect it would take a couple hours for me to do the necessary SVN checkouts and diff-minimization), but I would like to avoid seeing the overall bug here being cancelled.
        Hide
        Mahadev konar added a comment -

        Erik,
        thanks for your input. A cancel on patch available just means that it needs to be revised a little more for committing to trunk. It does not mean that the changes are unacceptable. With a few changes like you suggested we should be able to commit this to 3.4.

        Show
        Mahadev konar added a comment - Erik, thanks for your input. A cancel on patch available just means that it needs to be revised a little more for committing to trunk. It does not mean that the changes are unacceptable. With a few changes like you suggested we should be able to commit this to 3.4.
        Hide
        Dheeraj Agrawal added a comment -

        Mahadev, i will add details to README file on steps to get this compiled.
        Erik, i would like to get a gist of what changes were required to get it working? Were there any bugs, or were they issues while compiling it?

        Show
        Dheeraj Agrawal added a comment - Mahadev, i will add details to README file on steps to get this compiled. Erik, i would like to get a gist of what changes were required to get it working? Were there any bugs, or were they issues while compiling it?
        Hide
        Erik Anderson added a comment -

        There were a fair number of compiler issues, for instance "inline" is apparently not a keyword when compiling to C but "__inline" is. I also ended up changing the datatype for filedescriptor from "int" to "SOCKET" as I wasn't convinced there wouldn't be problems down the road.

        I am getting coredumps when running the CLI without a port number (inappropriate use of uninitialized pthread_cond object) and do get a fair number of CancelledKeyException errors, but other than that I think I have a working program.

        I was going to see if I could check out the 3.3 source in SVN so that I could get a better idea of the changes I made.

        Show
        Erik Anderson added a comment - There were a fair number of compiler issues, for instance "inline" is apparently not a keyword when compiling to C but "__inline" is. I also ended up changing the datatype for filedescriptor from "int" to "SOCKET" as I wasn't convinced there wouldn't be problems down the road. I am getting coredumps when running the CLI without a port number (inappropriate use of uninitialized pthread_cond object) and do get a fair number of CancelledKeyException errors, but other than that I think I have a working program. I was going to see if I could check out the 3.3 source in SVN so that I could get a better idea of the changes I made.
        Hide
        Dheeraj Agrawal added a comment -

        the core dump when you dont pass port number might be related to this issue i had raised: https://issues.apache.org/jira/browse/ZOOKEEPER-1029

        Show
        Dheeraj Agrawal added a comment - the core dump when you dont pass port number might be related to this issue i had raised: https://issues.apache.org/jira/browse/ZOOKEEPER-1029
        Erik Anderson made changes -
        Attachment win32-odysseus-vc2k5.patch [ 12479379 ]
        Hide
        Erik Anderson added a comment -

        Okay, I've attached what I came up with, unlikely to be considered for inclusion as-is (I prob have an awful lot of tabs in the attachment for instance). Prob more for Dheeraj to take a look at.

        Also, I couldn't figure out whether the 3.3.3 download I used was "release-3.3.3" or "release-3.3.3-rc0" so there may be some re-patching going on in here.

        Show
        Erik Anderson added a comment - Okay, I've attached what I came up with, unlikely to be considered for inclusion as-is (I prob have an awful lot of tabs in the attachment for instance). Prob more for Dheeraj to take a look at. Also, I couldn't figure out whether the 3.3.3 download I used was "release-3.3.3" or "release-3.3.3-rc0" so there may be some re-patching going on in here.
        Hide
        Dheeraj Agrawal added a comment -

        Most of the compile error were due to wrong values in config.h. I have changed that. will upload the new patch

        Show
        Dheeraj Agrawal added a comment - Most of the compile error were due to wrong values in config.h. I have changed that. will upload the new patch
        Hide
        Dheeraj Agrawal added a comment -

        Uploading the new patch. This time I have modified the jute generator code, to simplyfy the patch. (earlier patch included zookeeper.jute.h/c, this one doesnt)

        Steps to apply patch and build:
        1) apply patch to zk trunk. patch -p0 < patchfile
        2) There might be a conflict for zookeeper.h , manually change that file.
        3) run ant compile_jute to generate the zookeeper_jute.h/c
        4) Add ZOOKEEPER_HOME environment variable to your windows environment (My Computer->Properties->Advanced->Environment Variables->New) and point it to the directory where you have checked out the trunk. (C:\zookeeper-trunk)
        5) Open zookeeper.sln file in src/c (this is visual studio 2008 solution file)
        6) Right click on Cli project in the Solution Explorer, select Project Dependencies and make it depend on zookeeper project by checking zookeeper.
        7) Click on Build->Batch Build and check all the builds (cli - Debug/Release, zookeeper - Debug/Release) and hit Build

        To run the client:

        Right click on Cli project > Properties> Debugging > Command Arguments (enter the hostname:port for the zk server you want to connect to), save and then Debug>start Debugging.

        Show
        Dheeraj Agrawal added a comment - Uploading the new patch. This time I have modified the jute generator code, to simplyfy the patch. (earlier patch included zookeeper.jute.h/c, this one doesnt) Steps to apply patch and build: 1) apply patch to zk trunk. patch -p0 < patchfile 2) There might be a conflict for zookeeper.h , manually change that file. 3) run ant compile_jute to generate the zookeeper_jute.h/c 4) Add ZOOKEEPER_HOME environment variable to your windows environment (My Computer->Properties->Advanced->Environment Variables->New) and point it to the directory where you have checked out the trunk. (C:\zookeeper-trunk) 5) Open zookeeper.sln file in src/c (this is visual studio 2008 solution file) 6) Right click on Cli project in the Solution Explorer, select Project Dependencies and make it depend on zookeeper project by checking zookeeper. 7) Click on Build->Batch Build and check all the builds (cli - Debug/Release, zookeeper - Debug/Release) and hit Build To run the client: Right click on Cli project > Properties > Debugging > Command Arguments (enter the hostname:port for the zk server you want to connect to), save and then Debug >start Debugging.
        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER-992.patch [ 12481735 ]
        Hide
        Dheeraj Agrawal added a comment -

        Erik/Michi, you want to give this one a try?

        Show
        Dheeraj Agrawal added a comment - Erik/Michi, you want to give this one a try?
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12481735/ZOOKEEPER-992.patch
        against trunk revision 1132489.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/306//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/12481735/ZOOKEEPER-992.patch against trunk revision 1132489. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/306//console This message is automatically generated.
        Hide
        Dheeraj Agrawal added a comment -

        Uploaded the patch which doesnt need manual intervention to resolve conflict.

        Show
        Dheeraj Agrawal added a comment - Uploaded the patch which doesnt need manual intervention to resolve conflict.
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER-992-final.patch [ 12481743 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12481743/ZOOKEEPER-992-final.patch
        against trunk revision 1132489.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/307//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/12481743/ZOOKEEPER-992-final.patch against trunk revision 1132489. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/307//console This message is automatically generated.
        Hide
        Patrick Hunt added a comment -

        Hi, there still seems to be one problem, canceling while the patch is updated:

        [exec] patching file src/c/include/zookeeper.h
        [exec] Hunk #3 FAILED at 400.
        [exec] 1 out of 3 hunks FAILED – saving rejects to file src/c/include/zookeeper.h.rej

        Show
        Patrick Hunt added a comment - Hi, there still seems to be one problem, canceling while the patch is updated: [exec] patching file src/c/include/zookeeper.h [exec] Hunk #3 FAILED at 400. [exec] 1 out of 3 hunks FAILED – saving rejects to file src/c/include/zookeeper.h.rej
        Patrick Hunt made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER_992_final.patch [ 12481960 ]
        Hide
        Dheeraj Agrawal added a comment -

        updated the patch

        Show
        Dheeraj Agrawal added a comment - updated the patch
        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12481960/ZOOKEEPER_992_final.patch
        against trunk revision 1132489.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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 (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/309//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/309//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/309//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/12481960/ZOOKEEPER_992_final.patch against trunk revision 1132489. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/309//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/309//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/309//console This message is automatically generated.
        Dheeraj Agrawal made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER_992_FINAL.patch [ 12481978 ]
        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Patrick Hunt added a comment - - edited

        The compilation is now failing during c code compilation (unfortunately jenkins doesn't report this in testresults, you need to check the console):

             [exec]      [exec] /bin/bash ./libtool --tag=CC   --mode=compile gcc -DHAVE_CONFIG_H -I. -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c  -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/include -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/tests -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/generated  -Wall -Werror  -g -O2 -D_GNU_SOURCE -MT zookeeper.lo -MD -MP -MF .deps/zookeeper.Tpo -c -o zookeeper.lo `test -f 'src/zookeeper.c' || echo '/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/'`src/zookeeper.c
             [exec]      [exec] libtool: compile:  gcc -DHAVE_CONFIG_H -I. -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/include -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/tests -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/generated -Wall -Werror -g -O2 -D_GNU_SOURCE -MT zookeeper.lo -MD -MP -MF .deps/zookeeper.Tpo -c /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/src/zookeeper.c  -fPIC -DPIC -o .libs/zookeeper.o
             [exec]      [exec] In file included from /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/src/zookeeper.c:27:
             [exec]      [exec] /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/include/zookeeper.h:407: error: expected declaration specifiers or '...' before 'SOCKET'
             [exec]      [exec] /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/src/zookeeper.c:1494: error: conflicting types for 'zookeeper_interest'
             [exec]      [exec] /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/include/zookeeper.h:407: error: previous declaration of 'zookeeper_interest' was here
        
        Show
        Patrick Hunt added a comment - - edited The compilation is now failing during c code compilation (unfortunately jenkins doesn't report this in testresults, you need to check the console): [exec] [exec] /bin/bash ./libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/include -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/tests -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/generated -Wall -Werror -g -O2 -D_GNU_SOURCE -MT zookeeper.lo -MD -MP -MF .deps/zookeeper.Tpo -c -o zookeeper.lo `test -f 'src/zookeeper.c' || echo '/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/'`src/zookeeper.c [exec] [exec] libtool: compile: gcc -DHAVE_CONFIG_H -I. -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/include -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/tests -I/grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/generated -Wall -Werror -g -O2 -D_GNU_SOURCE -MT zookeeper.lo -MD -MP -MF .deps/zookeeper.Tpo -c /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/src/zookeeper.c -fPIC -DPIC -o .libs/zookeeper.o [exec] [exec] In file included from /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/src/zookeeper.c:27: [exec] [exec] /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/include/zookeeper.h:407: error: expected declaration specifiers or '...' before 'SOCKET' [exec] [exec] /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/src/zookeeper.c:1494: error: conflicting types for 'zookeeper_interest' [exec] [exec] /grid/0/hudson/hudson-slave/workspace/PreCommit-ZOOKEEPER-Build/trunk/src/c/include/zookeeper.h:407: error: previous declaration of 'zookeeper_interest' was here
        Patrick Hunt made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Dheeraj Agrawal added a comment -

        are you sure this is the output from latest patch. this was the failure from ZOOKEEPER_992_final.patch . After that i loaded ZOOKEEPER_992_FINAL.patch which fixed this issue.

        Show
        Dheeraj Agrawal added a comment - are you sure this is the output from latest patch. this was the failure from ZOOKEEPER_992_final.patch . After that i loaded ZOOKEEPER_992_FINAL.patch which fixed this issue.
        Hide
        Dheeraj Agrawal added a comment -

        i see it in the console. let me check whats going on. as i tested on linux and windows and it worked fine before uploading it.

        Show
        Dheeraj Agrawal added a comment - i see it in the console. let me check whats going on. as i tested on linux and windows and it worked fine before uploading it.
        Hide
        Dheeraj Agrawal added a comment -

        i had uploaded following patch after this error:
        ZOOKEEPER_992_FINAL.patch 09/Jun/11 21:03
        But i dont see QA run after that. The console output is from a old patch.

        Show
        Dheeraj Agrawal added a comment - i had uploaded following patch after this error: ZOOKEEPER_992_FINAL.patch 09/Jun/11 21:03 But i dont see QA run after that. The console output is from a old patch.
        Hide
        Patrick Hunt added a comment -
        Show
        Patrick Hunt added a comment - Ok, my fault, I just kicked off a new build here: https://builds.apache.org/view/S-Z/view/ZooKeeper/job/PreCommit-ZOOKEEPER-Build/312/
        Patrick Hunt made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Show
        Patrick Hunt added a comment - grr, here: https://builds.apache.org/view/S-Z/view/ZooKeeper/job/PreCommit-ZOOKEEPER-Build/313/
        Hide
        Patrick Hunt added a comment -

        for future reference: we typically name all the patches the same, jira handles it correctly (you can see all the older versions and it typically highlights the newest). Regards.

        Show
        Patrick Hunt added a comment - for future reference: we typically name all the patches the same, jira handles it correctly (you can see all the older versions and it typically highlights the newest). Regards.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12481978/ZOOKEEPER_992_FINAL.patch
        against trunk revision 1132489.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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 (version 1.3.9) 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: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/313//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/313//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/313//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/12481978/ZOOKEEPER_992_FINAL.patch against trunk revision 1132489. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 (version 1.3.9) 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: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/313//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/313//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/313//console This message is automatically generated.
        Hide
        Dheeraj Agrawal added a comment -

        Thanks Patrick, will keep that in mind..didnt know jira could handle that...

        Show
        Dheeraj Agrawal added a comment - Thanks Patrick, will keep that in mind..didnt know jira could handle that...
        Hide
        Michi Mutsuzaki added a comment -

        Hi Dheeraj,

        Finally I was able to compile it on windows. The patch looks good, except there is a typo (STRUCT_INTIALIZER is missing an I).

        Thanks!
        --Michi

        Show
        Michi Mutsuzaki added a comment - Hi Dheeraj, Finally I was able to compile it on windows. The patch looks good, except there is a typo (STRUCT_INTIALIZER is missing an I). Thanks! --Michi
        Hide
        Michi Mutsuzaki added a comment -

        By the way, do we have a windows build box? It would be great if we can run build/unit tests on a windows box automatically.

        --Michi

        Show
        Michi Mutsuzaki added a comment - By the way, do we have a windows build box? It would be great if we can run build/unit tests on a windows box automatically. --Michi
        Hide
        Michi Mutsuzaki (Inactive) added a comment -

        Camille, could you also review this patch?

        Thanks!
        --Michi

        Show
        Michi Mutsuzaki (Inactive) added a comment - Camille, could you also review this patch? Thanks! --Michi
        Hide
        Camille Fournier added a comment -

        This has a +1 from me, we have reviewed it internally and are already using it.

        Show
        Camille Fournier added a comment - This has a +1 from me, we have reviewed it internally and are already using it.
        Hide
        Michi Mutsuzaki (Inactive) added a comment -

        Oh that's great to know. I'll check it in one the typo is fixed.

        Thanks!
        --Michi

        Show
        Michi Mutsuzaki (Inactive) added a comment - Oh that's great to know. I'll check it in one the typo is fixed. Thanks! --Michi
        Hide
        Mahadev konar added a comment -

        Dheeraj/michi are we waiting on just a typo fix? I can do that if thats all we need to check this in?

        Show
        Mahadev konar added a comment - Dheeraj/michi are we waiting on just a typo fix? I can do that if thats all we need to check this in?
        Mahadev konar made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hide
        Mahadev konar added a comment -

        Reuploading Dheeraj's patch to trigger hudson.

        Show
        Mahadev konar added a comment - Reuploading Dheeraj's patch to trigger hudson.
        Mahadev konar made changes -
        Attachment ZOOKEEPER-992-final.patch [ 12485536 ]
        Mahadev konar made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12485536/ZOOKEEPER-992-final.patch
        against trunk revision 1142377.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/372//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/12485536/ZOOKEEPER-992-final.patch against trunk revision 1142377. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/372//console This message is automatically generated.
        Hide
        Mahadev konar added a comment -

        Looks like the patch does not apply any more with multi update checkins.

        Dheeraj, would you be able to update (this will be the last time ) the patch? I will commit as soon as you re upload!

        Show
        Mahadev konar added a comment - Looks like the patch does not apply any more with multi update checkins. Dheeraj, would you be able to update (this will be the last time ) the patch? I will commit as soon as you re upload!
        Hide
        Camille Fournier added a comment -

        Dheeraj is out this week, so this will have to wait for his return next week, but please do wait for it. Thanks.

        Show
        Camille Fournier added a comment - Dheeraj is out this week, so this will have to wait for his return next week, but please do wait for it. Thanks.
        Hide
        Erik Anderson added a comment -

        Breezing through again to take a look at this with my poor old VS2k5 machine, I was not able to compile the patch (likely just need a newer version of the generated files). Issues that I did run into though:

        • struct MultiHeader initializers (recently introduced) likely need to use STRUCT_INTIALIZER
        • I had to #define _WIN32_WINNT_WS03 and comment out an #include <SdkDdkVer.h> to get this to compile for me
        Show
        Erik Anderson added a comment - Breezing through again to take a look at this with my poor old VS2k5 machine, I was not able to compile the patch (likely just need a newer version of the generated files). Issues that I did run into though: struct MultiHeader initializers (recently introduced) likely need to use STRUCT_INTIALIZER I had to #define _WIN32_WINNT_WS03 and comment out an #include <SdkDdkVer.h> to get this to compile for me
        Hide
        Camille Fournier added a comment -

        Would love to have someone else help us with the last bits of this fix, if you have the time Erik.

        Show
        Camille Fournier added a comment - Would love to have someone else help us with the last bits of this fix, if you have the time Erik.
        Hide
        Erik Anderson added a comment -

        At the risk of resetting the clock on review, yanking deserved credit from someone, and rather making a mess of things, have attached my submission. Have not tested beyond running CLI and getting usage prompt

        Show
        Erik Anderson added a comment - At the risk of resetting the clock on review, yanking deserved credit from someone, and rather making a mess of things, have attached my submission. Have not tested beyond running CLI and getting usage prompt
        Erik Anderson made changes -
        Attachment ZOOKEEPER-992-round8.patch [ 12485810 ]
        Hide
        Erik Anderson added a comment -

        Another thing to point out in the patch I just uploaded, I did move calls to assert() below the point at which the variable they're asserting is dereferenced, which likely makes them useless. The C compiler was complaining about declarations following a non-declaration, so the code wouldn't be accepted as-is.

        Show
        Erik Anderson added a comment - Another thing to point out in the patch I just uploaded, I did move calls to assert() below the point at which the variable they're asserting is dereferenced, which likely makes them useless. The C compiler was complaining about declarations following a non-declaration, so the code wouldn't be accepted as-is.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12485810/ZOOKEEPER-992-round8.patch
        against trunk revision 1144087.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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 (version 1.3.9) warnings.

        -1 release audit. The applied patch generated 26 release audit warnings (more than the trunk's current 24 warnings).

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/387//testReport/
        Release audit warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/387//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/387//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/387//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/12485810/ZOOKEEPER-992-round8.patch against trunk revision 1144087. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 (version 1.3.9) warnings. -1 release audit. The applied patch generated 26 release audit warnings (more than the trunk's current 24 warnings). +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/387//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/387//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/387//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/387//console This message is automatically generated.
        Hide
        Camille Fournier added a comment -

        There's plenty of credit to go around for this one Erik, thanks for the help! Michi, can you take a look at the changes? I will also have Dheeraj check them out on Monday and make sure they work on our stack.

        Show
        Camille Fournier added a comment - There's plenty of credit to go around for this one Erik, thanks for the help! Michi, can you take a look at the changes? I will also have Dheeraj check them out on Monday and make sure they work on our stack.
        Hide
        Dheeraj Agrawal added a comment -

        Tested and reviewed the patch. looks good to me. Although it has vs2005 sln file, VS2008 easily converted it to the vs2008 sln format. so in a way its good to have vs2005 sln file.
        Regarding asserts, we can declared int rc; assert(req); rc = Request_path_init(...);
        This will maintain the sanity checks.

        After the patch is checked in, I can add notes to README (copied from above comments) on how to compile it on windows.
        We may need to point out on the definition of NTDDI_VERSION <which users can change based on there usage> in the README.

        Also when the patch is committed, can you do dos2unix on following files to get rid of Ctrl-M characters
        winconfig.h
        winstdint.h
        winport.c
        winport.h

        Show
        Dheeraj Agrawal added a comment - Tested and reviewed the patch. looks good to me. Although it has vs2005 sln file, VS2008 easily converted it to the vs2008 sln format. so in a way its good to have vs2005 sln file. Regarding asserts, we can declared int rc; assert(req); rc = Request_path_init(...); This will maintain the sanity checks. After the patch is checked in, I can add notes to README (copied from above comments) on how to compile it on windows. We may need to point out on the definition of NTDDI_VERSION <which users can change based on there usage> in the README. Also when the patch is committed, can you do dos2unix on following files to get rid of Ctrl-M characters winconfig.h winstdint.h winport.c winport.h
        Hide
        Dheeraj Agrawal added a comment -

        I have found a minor bug in this implementation and it shows up only during the retry logic of connecting to some other node in cluster and has to do with different return codes of send / recv between unix and windows. i will upload a new patch by tomorrow.

        Show
        Dheeraj Agrawal added a comment - I have found a minor bug in this implementation and it shows up only during the retry logic of connecting to some other node in cluster and has to do with different return codes of send / recv between unix and windows. i will upload a new patch by tomorrow.
        Hide
        Erik Anderson added a comment -

        Not sure of the relevancy, but the project files included in the patch are not the ones I intend to use locally. I will likely be building statically and the only option the project files provide is a dynamic (.dll) link. Is this something that people are going to run into (and not be able to fix themselves) enough to support here?

        Show
        Erik Anderson added a comment - Not sure of the relevancy, but the project files included in the patch are not the ones I intend to use locally. I will likely be building statically and the only option the project files provide is a dynamic (.dll) link. Is this something that people are going to run into (and not be able to fix themselves) enough to support here?
        Hide
        Dheeraj Agrawal added a comment -

        Fixed the typos, and the return code bugs. tested this on unix and windows.

        Show
        Dheeraj Agrawal added a comment - Fixed the typos, and the return code bugs. tested this on unix and windows.
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER-992.patch [ 12486693 ]
        Hide
        Mahadev konar added a comment - - edited

        THanks Dheeraj,
        Erik, can you give this patch a try and post your comments? Ill check this in if you are ok with the patch.

        Show
        Mahadev konar added a comment - - edited THanks Dheeraj, Erik, can you give this patch a try and post your comments? Ill check this in if you are ok with the patch.
        Hide
        Erik Anderson added a comment -

        If it gets uploaded, I'm getting close to actually beginning development around this, maybe a C++ wrapper of some kind.

        Show
        Erik Anderson added a comment - If it gets uploaded, I'm getting close to actually beginning development around this, maybe a C++ wrapper of some kind.
        Hide
        Camille Fournier added a comment -

        What's the status with this? We have tested it internally and it all looks good. Michi, are you still the committer for this?

        Show
        Camille Fournier added a comment - What's the status with this? We have tested it internally and it all looks good. Michi, are you still the committer for this?
        Hide
        Michi Mutsuzaki added a comment -

        I think it's good to go. Erik/Mahadev, are you ok with me checking this in?

        Show
        Michi Mutsuzaki added a comment - I think it's good to go. Erik/Mahadev, are you ok with me checking this in?
        Hide
        Erik Anderson added a comment -

        If you're talking about the patch dated July9 (which is the most recent patch I see), Dheeraj commented on July15 that he made some corrections and created a new patch that I'm not seeing here.

        I have no objections to a patch dated July15 or later being checked in.

        Show
        Erik Anderson added a comment - If you're talking about the patch dated July9 (which is the most recent patch I see), Dheeraj commented on July15 that he made some corrections and created a new patch that I'm not seeing here. I have no objections to a patch dated July15 or later being checked in.
        Hide
        Dheeraj Agrawal added a comment -

        Yes, the patch data July 15th is the latest one which includes some bug fixes. (specifically in send_buffer and recv_buffer methods and minor changes to pthread_self, pthread_equals implementation)

        1. ZOOKEEPER-992.patch 15/Jul/11 22:14 82 kB Dheeraj Agrawal
        Show
        Dheeraj Agrawal added a comment - Yes, the patch data July 15th is the latest one which includes some bug fixes. (specifically in send_buffer and recv_buffer methods and minor changes to pthread_self, pthread_equals implementation) ZOOKEEPER-992 .patch 15/Jul/11 22:14 82 kB Dheeraj Agrawal
        Hide
        Michi Mutsuzaki added a comment -

        Committed to trunk.

        --Michi

        Show
        Michi Mutsuzaki added a comment - Committed to trunk. --Michi
        Michi Mutsuzaki made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #1248 (See https://builds.apache.org/job/ZooKeeper-trunk/1248/)
        ZOOKEEPER-992. MT Native Version of Windows C Client (Dheeraj Agrawal via michim)

        michim : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1148116
        Files :

        • /zookeeper/trunk/src/c/src/zk_adaptor.h
        • /zookeeper/trunk/src/c/src/winport.c
        • /zookeeper/trunk/src/c/src/cli.c
        • /zookeeper/trunk/src/java/main/org/apache/jute/compiler/JRecord.java
        • /zookeeper/trunk/src/c/src/winport.h
        • /zookeeper/trunk/src/c/include/winconfig.h
        • /zookeeper/trunk/src/c/zookeeper.sln
        • /zookeeper/trunk/src/c/zookeeper.vcproj
        • /zookeeper/trunk/src/c/src/mt_adaptor.c
        • /zookeeper/trunk/src/c/src/zookeeper.c
        • /zookeeper/trunk/src/c/src/zk_hashtable.c
        • /zookeeper/trunk/src/c/include/winstdint.h
        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/c/src/zk_log.c
        • /zookeeper/trunk/src/c/include/zookeeper.h
        • /zookeeper/trunk/src/c/src/recordio.c
        • /zookeeper/trunk/src/c/src/load_gen.c
        • /zookeeper/trunk/src/c/src/hashtable/hashtable.h
        • /zookeeper/trunk/src/c/Cli.vcproj
        • /zookeeper/trunk/src/c/include/recordio.h
        • /zookeeper/trunk/src/c/src/hashtable/hashtable_itr.c
        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #1248 (See https://builds.apache.org/job/ZooKeeper-trunk/1248/ ) ZOOKEEPER-992 . MT Native Version of Windows C Client (Dheeraj Agrawal via michim) michim : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1148116 Files : /zookeeper/trunk/src/c/src/zk_adaptor.h /zookeeper/trunk/src/c/src/winport.c /zookeeper/trunk/src/c/src/cli.c /zookeeper/trunk/src/java/main/org/apache/jute/compiler/JRecord.java /zookeeper/trunk/src/c/src/winport.h /zookeeper/trunk/src/c/include/winconfig.h /zookeeper/trunk/src/c/zookeeper.sln /zookeeper/trunk/src/c/zookeeper.vcproj /zookeeper/trunk/src/c/src/mt_adaptor.c /zookeeper/trunk/src/c/src/zookeeper.c /zookeeper/trunk/src/c/src/zk_hashtable.c /zookeeper/trunk/src/c/include/winstdint.h /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/c/src/zk_log.c /zookeeper/trunk/src/c/include/zookeeper.h /zookeeper/trunk/src/c/src/recordio.c /zookeeper/trunk/src/c/src/load_gen.c /zookeeper/trunk/src/c/src/hashtable/hashtable.h /zookeeper/trunk/src/c/Cli.vcproj /zookeeper/trunk/src/c/include/recordio.h /zookeeper/trunk/src/c/src/hashtable/hashtable_itr.c
        Hide
        ZhangQi added a comment -

        i found a strange phenomenon when i test your zk trunk with a c program(build by visual studio 2003). I run the zk service in a 3 machine cluster.First i build a node /zookeeper/zk_test01. Then i insert sequence node /zookeeper/zk_test01/node. zk service will fail when the sequence node number exceed 200000, at the same time zkcli lost connection with zk service.How does it happen, does zk have node number limit?

        Show
        ZhangQi added a comment - i found a strange phenomenon when i test your zk trunk with a c program(build by visual studio 2003). I run the zk service in a 3 machine cluster.First i build a node /zookeeper/zk_test01. Then i insert sequence node /zookeeper/zk_test01/node. zk service will fail when the sequence node number exceed 200000, at the same time zkcli lost connection with zk service.How does it happen, does zk have node number limit?
        Hide
        Mahadev konar added a comment -

        Zhang,
        ZK has a limit on the amount of data that be transferred across the server and client (i.e 1MB). If you are creating direct children of single node and the data is more than 1MB, getChildren might fail. Also, please open a seperate jira to track this.

        Show
        Mahadev konar added a comment - Zhang, ZK has a limit on the amount of data that be transferred across the server and client (i.e 1MB). If you are creating direct children of single node and the data is more than 1MB, getChildren might fail. Also, please open a seperate jira to track this.
        Mahadev konar made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Patch Available Patch Available Open Open
        116d 3h 43m 5 Mahadev konar 07/Jul/11 07:18
        Open Open Patch Available Patch Available
        23d 10h 6m 6 Mahadev konar 07/Jul/11 07:19
        Patch Available Patch Available Resolved Resolved
        11d 18h 40m 1 Michi Mutsuzaki 19/Jul/11 01:59
        Resolved Resolved Closed Closed
        127d 18h 22m 1 Mahadev konar 23/Nov/11 19:22

          People

          • Assignee:
            Dheeraj Agrawal
            Reporter:
            Camille Fournier
          • Votes:
            2 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development