libmemcached

Merge lp://staging/~trond-norbye/libmemcached/bug_394442 into lp://staging/~tangent-org/libmemcached/trunk

  1. bug_394442
  2. Merge into trunk
Proposed by Trond Norbye
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~trond-norbye/libmemcached/bug_394442
Merge into: lp://staging/~tangent-org/libmemcached/trunk
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~trond-norbye/libmemcached/bug_394442
Reviewer Review Type Date Requested Status
Libmemcached-developers Pending
Review via email: mp+8098@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Trond Norbye (trond-norbye) wrote :

 Bug 394442: Replication mget test fail on linux due to race conditions

  All replicas are stored on the different servers by using the quiet commands,
  and when we start to receive them we will do that in another memcached_st
  instance (aka another connection to the server). The code goes directly
  from the SET command to trying to fetch the items from all of the servers.
  This means that the memcached server may still be processing the quiet set
  commands while another thread in the memcached server tries to fetch all of
  the items.

  To fix this we need to ensure that all of the set commands are executed on
  all of the memcached servers before starting to receive them. memcached_quit
  will send the QUIT command to all of the servers and wait for a response,
  so we now that when memcached_quit returns all commands are executed on
  the client.

  In addition the response counter should not be updated when we send out the
  replica storage / delete commands.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libmemcached/memcached_delete.c'
2--- libmemcached/memcached_delete.c 2009-06-21 15:51:17 +0000
3+++ libmemcached/memcached_delete.c 2009-07-01 19:36:25 +0000
4@@ -143,7 +143,9 @@
5 if ((memcached_do(server, (const char*)request.bytes,
6 sizeof(request.bytes), 0) != MEMCACHED_SUCCESS) ||
7 (memcached_io_write(server, key, key_length, flush) == -1))
8- memcached_io_reset(server);
9+ memcached_io_reset(server);
10+ else
11+ memcached_server_response_decrement(server);
12 }
13 }
14
15
16=== modified file 'libmemcached/memcached_storage.c'
17--- libmemcached/memcached_storage.c 2009-06-21 15:51:17 +0000
18+++ libmemcached/memcached_storage.c 2009-07-01 19:36:25 +0000
19@@ -491,7 +491,9 @@
20 send_length, 0) != MEMCACHED_SUCCESS) ||
21 (memcached_io_write(srv, key, key_length, 0) == -1) ||
22 (memcached_io_write(srv, value, value_length, flush) == -1))
23- memcached_io_reset(server);
24+ memcached_io_reset(srv);
25+ else
26+ memcached_server_response_decrement(srv);
27 }
28 }
29
30
31=== modified file 'tests/function.c'
32--- tests/function.c 2009-06-29 17:47:16 +0000
33+++ tests/function.c 2009-07-01 19:36:25 +0000
34@@ -3070,6 +3070,16 @@
35 return rc;
36 }
37
38+static memcached_return pre_replication_noblock(memcached_st *memc)
39+{
40+ memcached_return rc= MEMCACHED_FAILURE;
41+ if (pre_replication(memc) == MEMCACHED_SUCCESS &&
42+ pre_nonblock(memc) == MEMCACHED_SUCCESS)
43+ rc= MEMCACHED_SUCCESS;
44+
45+ return rc;
46+}
47+
48 static void my_free(memcached_st *ptr __attribute__((unused)), void *mem)
49 {
50 free(mem);
51@@ -3583,6 +3593,21 @@
52 assert(rc == MEMCACHED_SUCCESS);
53
54 /*
55+ ** We are using the quiet commands to store the replicas, so we need
56+ ** to ensure that all of them are processed before we can continue.
57+ ** In the test we go directly from storing the object to trying to
58+ ** receive the object from all of the different servers, so we
59+ ** could end up in a race condition (the memcached server hasn't yet
60+ ** processed the quiet command from the replication set when it process
61+ ** the request from the other client (created by the clone)). As a
62+ ** workaround for that we call memcached_quit to send the quit command
63+ ** to the server and wait for the response ;-) If you use the test code
64+ ** as an example for your own code, please note that you shouldn't need
65+ ** to do this ;-)
66+ */
67+ memcached_quit(memc);
68+
69+ /*
70 ** "bubba" should now be stored on all of our servers. We don't have an
71 ** easy to use API to address each individual server, so I'll just iterate
72 ** through a bunch of "master keys" and I should most likely hit all of the
73@@ -3653,6 +3678,21 @@
74 }
75
76 /*
77+ ** We are using the quiet commands to store the replicas, so we need
78+ ** to ensure that all of them are processed before we can continue.
79+ ** In the test we go directly from storing the object to trying to
80+ ** receive the object from all of the different servers, so we
81+ ** could end up in a race condition (the memcached server hasn't yet
82+ ** processed the quiet command from the replication set when it process
83+ ** the request from the other client (created by the clone)). As a
84+ ** workaround for that we call memcached_quit to send the quit command
85+ ** to the server and wait for the response ;-) If you use the test code
86+ ** as an example for your own code, please note that you shouldn't need
87+ ** to do this ;-)
88+ */
89+ memcached_quit(memc);
90+
91+ /*
92 * Don't do the following in your code. I am abusing the internal details
93 * within the library, and this is not a supported interface.
94 * This is to verify correct behavior in the library
95@@ -4588,6 +4628,7 @@
96 {"consistent_ketama_weighted", pre_behavior_ketama_weighted, 0, consistent_weighted_tests},
97 {"test_hashes", 0, 0, hash_tests},
98 {"replication", pre_replication, 0, replication_tests},
99+ {"replication_noblock", pre_replication_noblock, 0, replication_tests},
100 {0, 0, 0, 0}
101 };
102

Subscribers

People subscribed via source and target branches

to all changes: