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

Proposed by Clint Byrum on 2011-01-25
Status: Merged
Merged at revision: 377
Proposed branch: lp:~clint-fewbar/gearmand/fix-connection-failover
Merge into: lp:gearmand/1.0
Diff against target: 76 lines (+37/-6)
2 files modified
libgearman/connection.c (+6/-6)
tests/worker_test.c (+31/-0)
To merge this branch: bzr merge lp:~clint-fewbar/gearmand/fix-connection-failover
Reviewer Review Type Date Requested Status
Brian Aker (community) 2011-01-25 Needs Information on 2011-02-09
Review via email: mp+47438@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Brian Aker (brianaker) wrote :

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) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libgearman/connection.c'
2--- libgearman/connection.c 2010-04-04 21:27:36 +0000
3+++ libgearman/connection.c 2011-02-15 20:28:17 +0000
4@@ -553,17 +553,17 @@
5 case GEARMAN_CON_UNIVERSAL_CONNECTING:
6 while (1)
7 {
8- if (connection->revents & POLLOUT)
9- {
10- connection->state= GEARMAN_CON_UNIVERSAL_CONNECTED;
11- break;
12- }
13- else if (connection->revents & (POLLERR | POLLHUP | POLLNVAL))
14+ if (connection->revents & (POLLERR | POLLHUP | POLLNVAL))
15 {
16 connection->state= GEARMAN_CON_UNIVERSAL_CONNECT;
17 connection->addrinfo_next= connection->addrinfo_next->ai_next;
18 break;
19 }
20+ else if (connection->revents & POLLOUT)
21+ {
22+ connection->state= GEARMAN_CON_UNIVERSAL_CONNECTED;
23+ break;
24+ }
25
26 gret= gearman_connection_set_events(connection, POLLOUT);
27 if (gret != GEARMAN_SUCCESS)
28
29=== modified file 'tests/worker_test.c'
30--- tests/worker_test.c 2010-06-30 10:49:39 +0000
31+++ tests/worker_test.c 2011-02-15 20:28:17 +0000
32@@ -629,6 +629,36 @@
33 return TEST_SUCCESS;
34 }
35
36+static test_return_t gearman_worker_failover_test(void *object)
37+{
38+ gearman_worker_st *worker= (gearman_worker_st *)object;
39+ gearman_worker_st *cloned;
40+ gearman_return_t rc;
41+ const char *function_name= "_gearman_worker_add_function_worker_fn";
42+
43+ cloned= gearman_worker_clone(NULL, worker);
44+ gearman_worker_add_server(cloned, NULL, WORKER_TEST_PORT);
45+ gearman_worker_add_server(cloned, NULL, WORKER_TEST_PORT+1);
46+
47+ rc= gearman_worker_add_function(cloned,
48+ function_name,
49+ 0, _gearman_worker_add_function_worker_fn, NULL);
50+ test_truth(rc == GEARMAN_SUCCESS);
51+
52+ gearman_worker_set_timeout(cloned, 2);
53+
54+ rc= gearman_worker_work(cloned);
55+ test_truth(rc == GEARMAN_TIMEOUT);
56+
57+ /* Make sure we have remove worker function */
58+ rc= gearman_worker_unregister(cloned, function_name);
59+ test_truth(rc == GEARMAN_SUCCESS);
60+
61+ gearman_worker_free(cloned);
62+
63+ return TEST_SUCCESS;
64+}
65+
66 /*********************** World functions **************************************/
67
68 void *create(void *object __attribute__((unused)))
69@@ -695,6 +725,7 @@
70 {"gearman_worker_unregister_all", 0, gearman_worker_unregister_all_test },
71 {"gearman_worker_work with timout", 0, gearman_worker_work_with_test },
72 {"gearman_worker_context", 0, gearman_worker_context_test },
73+ {"gearman_worker_failover", 0, gearman_worker_failover_test },
74 {"echo_max", 0, echo_max_test },
75 {"abandoned_worker", 0, abandoned_worker_test },
76 {0, 0, 0}

Subscribers

People subscribed via source and target branches