Fix bugs of zookeeper resolver

1 files changed, 44 insertions, 48 deletions

diff --git a/src/core/client_config/resolvers/zookeeper_resolver.c b/src/core/client_config/resolvers/zookeeper_resolver.c
index e124de93f7..2682e29e86 100644
--- a/src/core/client_config/resolvers/zookeeper_resolver.c
+++ b/src/core/client_config/resolvers/zookeeper_resolver.c
@@ -45,7 +45,7 @@
 #include "src/core/support/string.h"
 #include "src/core/json/json.h"
 
-#define GRPC_MAX_ZOOKEEPER_SIZE 1024
+#define GRPC_MAX_ZOOKEEPER_BUFFER_SIZE 1024
 #define GRPC_ZOOKEEPER_TIMEOUT 15000
 #define GRPC_ZOOKEEPER_WATCH 0
 
@@ -81,9 +81,9 @@ typedef struct {
   zhandle_t *zookeeper_handle;
   /** zookeeper resolved addresses */
   grpc_resolved_addresses * resolved_addrs;
-  /** zookeeper total number of addresses to be resolved */
+  /** total number of addresses to be resolved */
   int resolved_total;
-  /** zookeeper resolved number of addresses */
+  /** number of addresses resolved */
   int resolved_num;
 } zookeeper_resolver;
 
@@ -181,12 +181,8 @@ static void zookeeper_dns_resolved(void *arg, grpc_resolved_addresses *addresses
   size_t i;
   zookeeper_resolver *r = arg;
   r->resolved_num++;
-  gpr_log(GPR_INFO, "log");
-  gpr_log(GPR_INFO, "%d", addresses->naddrs);
-  gpr_log(GPR_INFO, "%d", r->resolved_addrs->naddrs);
   r->resolved_addrs->addrs = gpr_realloc(r->resolved_addrs->addrs, 
                              sizeof(grpc_resolved_address) * (r->resolved_addrs->naddrs + addresses->naddrs));
-  gpr_log(GPR_INFO, "log");
   for (i = 0; i < addresses->naddrs; i++) {
     memcpy(r->resolved_addrs->addrs[i + r->resolved_addrs->naddrs].addr, addresses->addrs[i].addr, addresses->addrs[i].len);
     r->resolved_addrs->addrs[i + r->resolved_addrs->naddrs].len = addresses->addrs[i].len;
@@ -216,9 +212,13 @@ static char *zookeeper_parse_address(char *buffer, int buffer_len) {
     for (cur = json->child; cur != NULL; cur = cur->next) {
       if (!strcmp(cur->key, "host")) {
         host = (char *)cur->value;
+        if (port != NULL)
+          break;
       }
-      if (!strcmp(cur->key, "port")) {
+      else if (!strcmp(cur->key, "port")) {
         port = (char *)cur->value;
+        if (host != NULL)
+          break;
       }
     }
     if (host != NULL && port != NULL) {
@@ -228,41 +228,37 @@ static char *zookeeper_parse_address(char *buffer, int buffer_len) {
       strcat(address, ":");
       strcat(address, port);
       gpr_log(GPR_INFO, address);
-    } else {
-      gpr_log(GPR_ERROR, "Cannot resolve zookeeper address: no host or port");
     }
     grpc_json_destroy(json);
-  } else {
-    gpr_log(GPR_ERROR, "Cannot resolve zookeeper address: json parse error");
-  }
+  } 
+
   return address;
 }
 
 /** Resolve address by zookeeper */
 static void zookeeper_resolve_address(zookeeper_resolver *r) {
-  struct String_vector addresses;
-  int i;
+  struct String_vector children;
   int status;
+  int i;
 
-  char path[GRPC_MAX_ZOOKEEPER_SIZE];
-  char buffer[GRPC_MAX_ZOOKEEPER_SIZE];
+  char path[GRPC_MAX_ZOOKEEPER_BUFFER_SIZE];
+  char buffer[GRPC_MAX_ZOOKEEPER_BUFFER_SIZE];
   char *address;
   int buffer_len;
   
   r->resolved_addrs = NULL;
   r->resolved_total = 0;
   r->resolved_num = 0;
-  gpr_log(GPR_INFO, r->name);
 
   address = NULL;
-  memset(path, 0, GRPC_MAX_ZOOKEEPER_SIZE);
-  memset(buffer, 0, GRPC_MAX_ZOOKEEPER_SIZE);
-  buffer_len = GRPC_MAX_ZOOKEEPER_SIZE;
+  memset(path, 0, GRPC_MAX_ZOOKEEPER_BUFFER_SIZE);
+  memset(buffer, 0, GRPC_MAX_ZOOKEEPER_BUFFER_SIZE);
+  buffer_len = GRPC_MAX_ZOOKEEPER_BUFFER_SIZE;
 
   /** Read zookeeper node of given path r->name 
       If not containing address, read its children */
+  gpr_log(GPR_INFO, r->name);
   status = zoo_get(r->zookeeper_handle, r->name, GRPC_ZOOKEEPER_WATCH, buffer, &buffer_len, NULL);
-  status = 0;
   if (!status) {
     if (buffer_len > 0) {
       address = zookeeper_parse_address(buffer, buffer_len);
@@ -277,40 +273,44 @@ static void zookeeper_resolve_address(zookeeper_resolver *r) {
       }
     }
 
-    buffer_len = GRPC_MAX_ZOOKEEPER_SIZE;
-    status = zoo_get_children(r->zookeeper_handle, r->name, GRPC_ZOOKEEPER_WATCH, &addresses);
+    buffer_len = GRPC_MAX_ZOOKEEPER_BUFFER_SIZE;
+    status = zoo_get_children(r->zookeeper_handle, r->name, GRPC_ZOOKEEPER_WATCH, &children);
     if (!status) {
-      /** Assume no children are deleted */
       r->resolved_addrs = gpr_malloc(sizeof(grpc_resolved_addresses));
       r->resolved_addrs->addrs = NULL;
       r->resolved_addrs->naddrs = 0;
-      r->resolved_total = addresses.count;
-      for (i = 0; i < addresses.count; i++) {
-        memset(path, 0, GRPC_MAX_ZOOKEEPER_SIZE);
+      r->resolved_total = children.count;
+
+      for (i = 0; i < children.count; i++) {
+        memset(path, 0, GRPC_MAX_ZOOKEEPER_BUFFER_SIZE);
         strcat(path, r->name);
         strcat(path, "/");
-        strcat(path, addresses.data[i]);
-        
+        strcat(path, children.data[i]);
         gpr_log(GPR_INFO, path);
-        memset(buffer, 0, GRPC_MAX_ZOOKEEPER_SIZE);
+        memset(buffer, 0, GRPC_MAX_ZOOKEEPER_BUFFER_SIZE);
         status = zoo_get(r->zookeeper_handle, path, GRPC_ZOOKEEPER_WATCH, buffer, &buffer_len, NULL);
+
         if (!status) {
           if (buffer_len > 0) {
             address = zookeeper_parse_address(buffer, buffer_len);
-            if (address != NULL) {
+            if (address != NULL)
               grpc_resolve_address(address, NULL, zookeeper_dns_resolved, r); 
-            }
+            else
+              gpr_log(GPR_ERROR, "Error when parsing zookeeper address %s", buffer);
           }
-        } else {
-          gpr_log(GPR_ERROR, "Cannot resolve zookeeper address: read zookeeper node error");
+        } else 
+          gpr_log(GPR_ERROR, "Error when reading zookeeper node %s", path);
+
+        if (address == NULL) {
+          r->resolved_total--;
+          if (r->resolved_num == r->resolved_total)
+            zookeeper_on_resolved(r, r->resolved_addrs);
         }
       }
-    } else {
-      gpr_log(GPR_ERROR, "Cannot resolve zookeeper address: get zookeeper children error");
-    }
-  } else {
-    gpr_log(GPR_ERROR, "Cannot resolve zookeeper address");
-  }
+    } else 
+      gpr_log(GPR_ERROR, "Error when getting children of zookeeper node %s", r->name);
+  } else
+    gpr_log(GPR_ERROR, "Error when reading zookeeper node %s", r->name);
   
   gpr_free(address);
 }
@@ -347,10 +347,8 @@ static void zookeeper_destroy(grpc_resolver *gr) {
   gpr_free(r);
 }
 
-/** Zookeeper watcher function: handle any updates to watched nodes */
-static void zookeeper_watcher(zhandle_t *zookeeper_handle, int type, int state, const char* path, void* watcher_ctx) {
-
-}
+/** Zookeeper watcher function - handle any updates to watched nodes */
+static void zookeeper_watcher(zhandle_t *zookeeper_handle, int type, int state, const char* path, void* watcher_ctx) {}
 
 static grpc_resolver *zookeeper_create(
     grpc_uri *uri,
@@ -359,8 +357,6 @@ static grpc_resolver *zookeeper_create(
     grpc_subchannel_factory *subchannel_factory) {
   zookeeper_resolver *r;
   const char *path = uri->path;
-  gpr_log(GPR_INFO, path);
-
   if (0 == strcmp(uri->authority, "")) {
     gpr_log(GPR_ERROR, "no authority specified in zookeeper uri");
     return NULL;
@@ -380,7 +376,7 @@ static grpc_resolver *zookeeper_create(
   zoo_set_debug_level(ZOO_LOG_LEVEL_WARN);
   r->zookeeper_handle = zookeeper_init(uri->authority, zookeeper_watcher, GRPC_ZOOKEEPER_TIMEOUT, 0, 0, 0);
   if (r->zookeeper_handle  == NULL) {
-    gpr_log(GPR_ERROR, "Cannot connect to zookeeper servers");
+    gpr_log(GPR_ERROR, "Error when connecting to zookeeper servers");
     return NULL;
   }