ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1020

Implement function in C client to determine which host you're currently connected to.

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.4.0
    • Component/s: c client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      On occasion it might be useful to determine which host your Zookeeper client is currently connected to, be it for debugging purposes or otherwise. A possible signature for that function:

      const char* zoo_get_connected_host(zhandle_t *zh, char *buffer, size_t buffer_size, unsigned short *port);

      Clients could use it like below:

      char buffer[33];
      unsigned short port = 0;
      if (!zoo_get_connected_host(zh, buffer, sizeof(buffer), &port))
      return EXIT_FAILURE;

      printf("The connected host is: %s:%d\n", buffer, port);

        Activity

        Stephen Tyree created issue -
        Hide
        Stephen Tyree added a comment -

        Patch to add functionality.

        Show
        Stephen Tyree added a comment - Patch to add functionality.
        Stephen Tyree made changes -
        Field Original Value New Value
        Attachment ZOOKEEPER-1020.patch [ 12473686 ]
        Hide
        Stephen Tyree added a comment -

        This time with proper spacing.

        Show
        Stephen Tyree added a comment - This time with proper spacing.
        Stephen Tyree made changes -
        Attachment ZOOKEEPER-1020.patch [ 12473687 ]
        Stephen Tyree made changes -
        Attachment ZOOKEEPER-1020.patch [ 12473686 ]
        Hide
        Stephen Tyree added a comment -

        Posting discussion with Benjamin Reed while on the IRC channel:

        13:19 < breed_zk> it looks good, but i think it might be better to use sockaddr instead of converting to a string. we should return a null if we aren't connected. your
        patch really tells the host that you should be connected to rather than which one you are connected to. maybe use getpeername
        13:21 < styree> Ok, so the function would return the sockaddr of the connection, letting the client do any conversion they want to?
        13:21 < styree> And calling getpeername on the fd used to talk to Zookeeper
        13:24 < styree> Using the sockaddr would remove the need to seperate the function into getting the host and the port, but how do we feel about the function name?
        13:31 < breed_zk> yes
        13:32 < breed_zk> one sec
        13:32 < styree> k
        13:35 < breed_zk> there is this, perhaps silly, distinction we make with the naming in C. zoo_ is for standard zookeeper calls, where zookeeper_ is for the management
        API, so by that logic i think it should be called zookeeper_get_connected_host.

        So I'm going to rename the function zookeeper_get_connected_host and have it return the underlying sockaddr for the connection based on getpeername.

        Show
        Stephen Tyree added a comment - Posting discussion with Benjamin Reed while on the IRC channel: 13:19 < breed_zk> it looks good, but i think it might be better to use sockaddr instead of converting to a string. we should return a null if we aren't connected. your patch really tells the host that you should be connected to rather than which one you are connected to. maybe use getpeername 13:21 < styree> Ok, so the function would return the sockaddr of the connection, letting the client do any conversion they want to? 13:21 < styree> And calling getpeername on the fd used to talk to Zookeeper 13:24 < styree> Using the sockaddr would remove the need to seperate the function into getting the host and the port, but how do we feel about the function name? 13:31 < breed_zk> yes 13:32 < breed_zk> one sec 13:32 < styree> k 13:35 < breed_zk> there is this, perhaps silly, distinction we make with the naming in C. zoo_ is for standard zookeeper calls, where zookeeper_ is for the management API, so by that logic i think it should be called zookeeper_get_connected_host. So I'm going to rename the function zookeeper_get_connected_host and have it return the underlying sockaddr for the connection based on getpeername.
        Hide
        Stephen Tyree added a comment -

        Updated the patch based on Benjamin Reed's comments.

        Show
        Stephen Tyree added a comment - Updated the patch based on Benjamin Reed's comments.
        Stephen Tyree made changes -
        Attachment ZOOKEEPER-1020.patch [ 12473721 ]
        Stephen Tyree made changes -
        Attachment ZOOKEEPER-1020.patch [ 12473687 ]
        Hide
        Benjamin Reed added a comment -

        +1 looks good. submitting to hudson. i'm fine with the name. if anyone would like a better name, please speak up! fyi, the new API is zookeeper_get_connected_host

        Show
        Benjamin Reed added a comment - +1 looks good. submitting to hudson. i'm fine with the name. if anyone would like a better name, please speak up! fyi, the new API is zookeeper_get_connected_host
        Benjamin Reed made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hadoop Flags [Reviewed]
        Hide
        Hadoop QA added a comment -

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

        +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://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/189//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/12473721/ZOOKEEPER-1020.patch against trunk revision 1081936. +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://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/189//console This message is automatically generated.
        Hide
        Stephen Tyree added a comment -

        This time the diff is from the root directory of trunk.

        Show
        Stephen Tyree added a comment - This time the diff is from the root directory of trunk.
        Stephen Tyree made changes -
        Attachment ZOOKEEPER-1020.patch [ 12473800 ]
        Stephen Tyree made changes -
        Attachment ZOOKEEPER-1020.patch [ 12473721 ]
        Hide
        Hadoop QA added a comment -

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

        +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://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/191//testReport/
        Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/191//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/191//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/12473800/ZOOKEEPER-1020.patch against trunk revision 1081936. +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://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/191//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/191//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/191//console This message is automatically generated.
        Hide
        Benjamin Reed added a comment -

        hadoopqa is right. we need a test. you could add a test to TestClient.cc or even just modify testAuth in that code to call zookeeper_get_connected_host before and after the stopServer calls to check both the connected and disconnected case.

        Show
        Benjamin Reed added a comment - hadoopqa is right. we need a test. you could add a test to TestClient.cc or even just modify testAuth in that code to call zookeeper_get_connected_host before and after the stopServer calls to check both the connected and disconnected case.
        Hide
        Stephen Tyree added a comment -

        This time with a unit test.

        Show
        Stephen Tyree added a comment - This time with a unit test.
        Stephen Tyree made changes -
        Attachment ZOOKEEPER-1020.patch [ 12473832 ]
        Stephen Tyree made changes -
        Attachment ZOOKEEPER-1020.patch [ 12473800 ]
        Hide
        Hadoop QA added a comment -

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

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

        +1 tests included. The patch appears to include 3 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 (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://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/193//testReport/
        Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/193//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/193//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/12473832/ZOOKEEPER-1020.patch against trunk revision 1082260. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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 (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://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/193//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/193//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/193//console This message is automatically generated.
        Hide
        Benjamin Reed added a comment -

        Committed revision 1082362.

        Show
        Benjamin Reed added a comment - Committed revision 1082362.
        Benjamin Reed made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #1124 (See https://hudson.apache.org/hudson/job/ZooKeeper-trunk/1124/)
        ZOOKEEPER-1020. Implement function in C client to determine which host you're currently connected to.

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #1124 (See https://hudson.apache.org/hudson/job/ZooKeeper-trunk/1124/ ) ZOOKEEPER-1020 . Implement function in C client to determine which host you're currently connected to.
        Patrick Hunt made changes -
        Assignee Stephen Tyree [ tyree731 ]
        Patrick Hunt made changes -
        Fix Version/s 3.4.0 [ 12314469 ]
        Mahadev konar made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Stephen Tyree
            Reporter:
            Stephen Tyree
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development