Zookeeper name resolver #2549

hongweiwang · 2015-07-20T21:35:07Z

Add zookeeper name resolver to gRPC. Zookeeper names in gRPC are represented as a URI.
zookeeper://host:port/name
Where host:port is zookeeper server address. name is zookeeper path of the service. An example would be zookeeper://127.0.0.1:2181/storage/mysql.

Services are registered in zookeeper server in hierarchical format service/instance, similar to Apache Curator. Each service is a zookeeper node, and each instance is a child node of the service. Each instance node stores its address (host and port) in JSON format.

If a node does not contain IP or port, consider its children.

grpc-kokoro · 2015-07-20T21:35:56Z

Can one of the admins verify this patch?

nicolasnoble · 2015-07-20T22:53:49Z

This is ok to test.

Also, you'll need to merge master again, it seems :)

ctiller · 2015-07-21T04:29:25Z

src/core/client_config/resolvers/zookeeper_resolver.c

+
+  /* Get zookeeper node of given path r->name 
+     If not containing address(i.e. service node), get its children */
+  status = zoo_get(r->zookeeper_handle, r->name, GRPC_ZOOKEEPER_WATCH, 


is this blocking?

Currently we use zookeeper synchronous API (zoo_get), which is blocking. We can avoid it by using asynchronous API.

ctiller · 2015-07-21T23:48:03Z

test this please

ctiller · 2015-07-21T23:48:37Z

This is ok to test.

ctiller · 2015-07-22T21:58:30Z

David - can you do a first pass review over this?

dgquintas · 2015-07-22T22:01:09Z

On it.

dgquintas · 2015-07-22T22:08:18Z

include/grpc/grpc_zookeeper.h

+extern "C" {
+#endif
+
+/* Register zookeeper name resolver in grpc */


tiny nit: please use /** so that the comment is picked up by doxygen.

dgquintas · 2015-08-13T14:44:08Z

include/grpc/grpc.h

@@ -355,6 +355,20 @@ typedef struct grpc_op {
  } data;
 } grpc_op;

+
+/** Registers a plugin to be initialized and deinitialized with the library.


what does "deinitialized" mean? Destroyed? Unregistered?

change "deinitialized" to "destroyed". Same for the argument "deinit" -> "destroy"

Replace all but the intro line with the following:

The \a init and \a destroy functions will be invoked as part of \a grpc_init() and \a grpc_shutdown(), respectively. Note that these functions can be invoked an arbitrary number of times (and hence so will \a init and \a destroy).
It is safe to pass NULL to either argument. Plugins are destroyed in the reverse order they were initialized.

The last line is only true if you implement Nico's comment to use an array instead of the current linked list to hold the registered plugins.

dgquintas · 2015-08-13T15:36:04Z

I've gone over the changes again. Sorry for the delay and thanks for pinging me about it :)

It looks good overall. I've added some minor comments.

nicolasnoble · 2015-08-13T17:25:59Z

src/core/surface/init.c

+  g_plugins_head->next = old_head;
+}
+
+void grpc_unregister_all_plugins() {


Let's actually pull that, and use a static array for the plugins instead of allocating them. 128 entries should be more than enough without wasting memory.

dgquintas · 2015-08-14T04:36:11Z

LGTM. Any last words before I merge? :)

Zookeeper name resolver

stanley-cheung · 2015-08-14T17:20:17Z

When I try to do a sudo make install after make in the base grpc directory, I got this error

[INSTALL] Installing public C headers
[MAKE]    Generating cache.mk
[STRIP]   Stripping libgpr.a
[STRIP]   Stripping libgrpc.a
[STRIP]   Stripping libgrpc_unsecure.a
[STRIP]   Stripping libgrpc_zookeeper.a
strip: './libs/opt/libgrpc_zookeeper.a': No such file
make: *** [strip-static_c] Error 1

hongweiwang · 2015-08-14T18:00:15Z

Fixed by #2941

jtattermusch · 2015-08-18T18:43:22Z

This PR have actually broken all Windows builds for 4 days. The error from Jenkins looks quite obviously related to the changes from this PR. Please check Jenkins status more carefully next time.

"c:\jenkins\workspace\gRPC_master\config\dbg\language\csharp\platform\windows\vsprojects\grpc_zookeeper\grpc_zookeeper.vcxproj.metaproj" (default target) (7) ->

(Build target) -> 

  c:\jenkins\workspace\gRPC_master\config\dbg\language\csharp\platform\windows\vsprojects\grpc_zookeeper\grpc_zookeeper.vcxproj.metaproj : error MSB3202: The project file "c:\jenkins\workspace\gRPC_master\config\dbg\language\csharp\platform\windows\vsprojects\grpc_zookeeper\grpc_zookeeper.vcxproj" was not found.

hongweiwang · 2015-08-18T19:12:52Z

I am sorry about it. @nicolasnoble helps me on grpc_zookeeper building. Please contact him to fix it.

huidi7 · 2016-08-09T02:20:02Z

@hongweiwang Hi, it seems there is limitation that the server side process has to be launched before the client when using zookeeper address for the service. If we launch the client side first, the zookeeper resolver will not try to resolve the service address any more when it finds there is no child node under the service zookeeper path. Just wonder if there is any incoming update on this.

thelinuxfoundation · 2019-01-26T18:20:04Z

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

User @hongweiwang isn't covered by a CLA. They will need to complete the form at https://identity.linuxfoundation.org/projects/cncf

Regards,
CLA GitHub bot

hongweiwang added 13 commits July 8, 2015 15:26

Add zookeeper resolver

275860b

Add zookeeper resolver

b1bbf6b

Zookeeper resolver works

85fd2f7

Support JSON format of Zookeeper node.

d65009d

If a node does not contain IP or port, consider its children.

Fix bugs of zookeeper resolver

c4cdbcd

Add grpc_zookeeper in build.json

da8858f

Add zookeeper library in Makefile template

a8cc439

Add zookeeper public header

be447a8

Register zookeeper plugin in grpc

a3780a8

Update uri_parser

f0e9fec

Zookeeper path remove the last slash

eec3b91

Merge branch 'master' into zookeeper

b307ae2

Fix lock

895b1f4

googlebot added the cla: yes label Jul 20, 2015

Minor fix on uri_parser_test

479532a

Merge branch 'master' of https://github.com/grpc/grpc into zookeeper

60db671

hongweiwang changed the title ~~Zookeeper Name Resolver~~ Zookeeper name resolver Jul 21, 2015

ctiller reviewed Jul 21, 2015
View reviewed changes

hongweiwang added 3 commits July 21, 2015 14:31

Change zookeeper API into asynchronous

3e50659

Minor fix on comments

ca2e32d

Fix memory leak by free address.

00de698

ctiller assigned dgquintas Jul 22, 2015

Formatting

cd62075

dgquintas reviewed Jul 22, 2015
View reviewed changes

hongweiwang added 5 commits August 10, 2015 18:26

Merge branch 'master' of https://github.com/grpc/grpc into zookeeper

ac133bd

Formatting zookeeper_test

d7c5aa8

Minor fix for debugging

eeb61ba

Debugging

31efd2a

Fix zookeeper resolver plugin bugs

ff6097a

dgquintas reviewed Aug 13, 2015
View reviewed changes

nicolasnoble reviewed Aug 13, 2015
View reviewed changes

hongweiwang added 2 commits August 13, 2015 16:13

Address comments and update grpc plugins

85ad685

Minor fix on comments

c3f48c8

dgquintas added a commit that referenced this pull request Aug 14, 2015

Merge pull request #2549 from hongweiwang/zookeeper

bd0d1bd

Zookeeper name resolver

dgquintas merged commit bd0d1bd into grpc:master Aug 14, 2015

lock bot locked as resolved and limited conversation to collaborators Jan 26, 2019

lock bot unassigned dgquintas Jan 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Zookeeper name resolver #2549

Zookeeper name resolver #2549

hongweiwang commented Jul 20, 2015

grpc-kokoro commented Jul 20, 2015

nicolasnoble commented Jul 20, 2015

ctiller Jul 21, 2015

hongweiwang Jul 21, 2015

ctiller commented Jul 21, 2015

ctiller commented Jul 21, 2015

ctiller commented Jul 22, 2015

dgquintas commented Jul 22, 2015

dgquintas Jul 22, 2015

dgquintas Aug 13, 2015

dgquintas Aug 13, 2015

dgquintas commented Aug 13, 2015

nicolasnoble Aug 13, 2015

dgquintas commented Aug 14, 2015

stanley-cheung commented Aug 14, 2015

hongweiwang commented Aug 14, 2015

jtattermusch commented Aug 18, 2015

hongweiwang commented Aug 18, 2015

huidi7 commented Aug 9, 2016

thelinuxfoundation commented Jan 26, 2019

Zookeeper name resolver #2549

Zookeeper name resolver #2549

Conversation

hongweiwang commented Jul 20, 2015

grpc-kokoro commented Jul 20, 2015

nicolasnoble commented Jul 20, 2015

ctiller Jul 21, 2015

Choose a reason for hiding this comment

hongweiwang Jul 21, 2015

Choose a reason for hiding this comment

ctiller commented Jul 21, 2015

ctiller commented Jul 21, 2015

ctiller commented Jul 22, 2015

dgquintas commented Jul 22, 2015

dgquintas Jul 22, 2015

Choose a reason for hiding this comment

dgquintas Aug 13, 2015

Choose a reason for hiding this comment

dgquintas Aug 13, 2015

Choose a reason for hiding this comment

dgquintas commented Aug 13, 2015

nicolasnoble Aug 13, 2015

Choose a reason for hiding this comment

dgquintas commented Aug 14, 2015

stanley-cheung commented Aug 14, 2015

hongweiwang commented Aug 14, 2015

jtattermusch commented Aug 18, 2015

hongweiwang commented Aug 18, 2015

huidi7 commented Aug 9, 2016

thelinuxfoundation commented Jan 26, 2019