Merge lp:~clint-fewbar/gearmand/fix-connection-failover into lp:gearmand/1.0

[Clint Byrum]

Fixes order of operations when errors happen to allow proper failover to other servers during CONNECT phase.

[Brian Aker (brianaker)]

Why comment out these lines?

 + //gearman_return_t ret= gearman_worker_echo(cloned, "test", sizeof("test"));
62 +
63 + //test_truth(ret == GEARMAN_SUCCESS);

review: Needs Information

[Clint Byrum (clint-fewbar)]

Oops sorry Brian for the slow response. I initially wrote the test to do an echo, but that wasn't actually testing the same code path (its more client-like than worker-like). Forgot to remove these two lines. I just pushed up a change which does just that.

=== modified file 'libgearman/connection.c'
--- libgearman/connection.c	2010-04-04 21:27:36 +0000
+++ libgearman/connection.c	2011-02-15 20:28:17 +0000
@@ -553,17 +553,17 @@
     case GEARMAN_CON_UNIVERSAL_CONNECTING:
       while (1)
       {
-        if (connection->revents & POLLOUT)
-        {
-          connection->state= GEARMAN_CON_UNIVERSAL_CONNECTED;
-          break;
-        }
-        else if (connection->revents & (POLLERR | POLLHUP | POLLNVAL))
+        if (connection->revents & (POLLERR | POLLHUP | POLLNVAL))
         {
           connection->state= GEARMAN_CON_UNIVERSAL_CONNECT;
           connection->addrinfo_next= connection->addrinfo_next->ai_next;
           break;
         }
+        else if (connection->revents & POLLOUT)
+        {
+          connection->state= GEARMAN_CON_UNIVERSAL_CONNECTED;
+          break;
+        }
 
         gret= gearman_connection_set_events(connection, POLLOUT);
         if (gret != GEARMAN_SUCCESS)

=== modified file 'tests/worker_test.c'
--- tests/worker_test.c	2010-06-30 10:49:39 +0000
+++ tests/worker_test.c	2011-02-15 20:28:17 +0000
@@ -629,6 +629,36 @@
   return TEST_SUCCESS;
 }
 
+static test_return_t gearman_worker_failover_test(void *object)
+{
+  gearman_worker_st *worker= (gearman_worker_st *)object;
+  gearman_worker_st *cloned;
+  gearman_return_t rc;
+  const char *function_name= "_gearman_worker_add_function_worker_fn";
+
+  cloned= gearman_worker_clone(NULL, worker);
+  gearman_worker_add_server(cloned, NULL, WORKER_TEST_PORT);
+  gearman_worker_add_server(cloned, NULL, WORKER_TEST_PORT+1);
+
+  rc= gearman_worker_add_function(cloned,
+				  function_name,
+				  0, _gearman_worker_add_function_worker_fn, NULL);
+  test_truth(rc == GEARMAN_SUCCESS);
+
+  gearman_worker_set_timeout(cloned, 2);
+
+  rc= gearman_worker_work(cloned);
+  test_truth(rc == GEARMAN_TIMEOUT);
+
+  /* Make sure we have remove worker function */
+  rc= gearman_worker_unregister(cloned, function_name);
+  test_truth(rc == GEARMAN_SUCCESS);
+
+  gearman_worker_free(cloned);
+
+  return TEST_SUCCESS;
+}
+
 /*********************** World functions **************************************/
 
 void *create(void *object __attribute__((unused)))
@@ -695,6 +725,7 @@
   {"gearman_worker_unregister_all", 0, gearman_worker_unregister_all_test },
   {"gearman_worker_work with timout", 0, gearman_worker_work_with_test },
   {"gearman_worker_context", 0, gearman_worker_context_test },
+  {"gearman_worker_failover", 0, gearman_worker_failover_test },
   {"echo_max", 0, echo_max_test },
   {"abandoned_worker", 0, abandoned_worker_test },
   {0, 0, 0}