Merge lp://qastaging/~jc.redoutey/libmemcached/mget_execute into lp://qastaging/~tangent-org/libmemcached/trunk

Proposed by JC Redoutey
Status: Merged
Merged at revision: not available
Proposed branch: lp://qastaging/~jc.redoutey/libmemcached/mget_execute
Merge into: lp://qastaging/~tangent-org/libmemcached/trunk
Diff against target: 162 lines (+92/-3)
4 files modified
libmemcached/memcached.h (+1/-0)
libmemcached/memcached_io.c (+5/-0)
libmemcached/memcached_response.c (+1/-1)
tests/function.c (+85/-2)
To merge this branch: bzr merge lp://qastaging/~jc.redoutey/libmemcached/mget_execute
Reviewer Review Type Date Requested Status
Libmemcached-developers Pending
Review via email: mp+15450@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
JC Redoutey (jc.redoutey) wrote :

New version with the changes asked
cheers

Revision history for this message
Brian Aker (brianaker) wrote :

I am looking at porting this forward right now.

A couple of things:

1) When you modify memcached_st to add a new member, you need to make sure it is initialized correctly.
2) Mixing int with size_t will just cause porting issues (this was in function.c).

I need to give a better example of how to use just one server during a test.

Thanks for this work, I am sorry I didn't get it in faster.

Cheers,
   -Brian

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libmemcached/memcached.h'
2--- libmemcached/memcached.h 2009-11-09 19:19:38 +0000
3+++ libmemcached/memcached.h 2009-11-30 21:30:22 +0000
4@@ -75,6 +75,7 @@
5
6 struct memcached_st {
7 uint8_t purging;
8+ uint8_t processing_input;
9 bool is_allocated;
10 uint8_t distribution;
11 uint8_t hash;
12
13=== modified file 'libmemcached/memcached_io.c'
14--- libmemcached/memcached_io.c 2009-10-14 11:40:42 +0000
15+++ libmemcached/memcached_io.c 2009-11-30 21:30:22 +0000
16@@ -120,10 +120,15 @@
17 */
18 memcached_callback_st cb= *ptr->root->callbacks;
19
20+ ptr->root->processing_input = 1;
21+
22 char buffer[MEMCACHED_DEFAULT_COMMAND_SIZE];
23 memcached_return error;
24 error= memcached_response(ptr, buffer, sizeof(buffer),
25 &ptr->root->result);
26+
27+ ptr->root->processing_input = 0;
28+
29 if (error == MEMCACHED_SUCCESS)
30 {
31 for (unsigned int x= 0; x < cb.number_of_callback; x++)
32
33=== modified file 'libmemcached/memcached_response.c'
34--- libmemcached/memcached_response.c 2009-10-06 10:48:57 +0000
35+++ libmemcached/memcached_response.c 2009-11-30 21:30:22 +0000
36@@ -44,7 +44,7 @@
37 memcached_result_st *result)
38 {
39 /* We may have old commands in the buffer not set, first purge */
40- if (ptr->root->flags & MEM_NO_BLOCK)
41+ if ((ptr->root->flags & MEM_NO_BLOCK) && (!ptr->root->processing_input))
42 (void)memcached_io_write(ptr, NULL, 0, 1);
43
44 /*
45
46=== modified file 'tests/function.c'
47--- tests/function.c 2009-11-27 17:05:51 +0000
48+++ tests/function.c 2009-11-30 21:30:22 +0000
49@@ -3922,7 +3922,7 @@
50 }
51
52 #ifdef HAVE_LIBMEMCACHEDUTIL
53-static void* connection_release(void *arg)
54+static void* connection_release(void *arg)
55 {
56 struct {
57 memcached_pool_st* pool;
58@@ -4169,7 +4169,7 @@
59 rc= memcached_set(memc, keys[x], len[x], "1", 1, 0, 0);
60 test_truth(rc == MEMCACHED_SUCCESS);
61 }
62-
63+
64 memcached_quit(memc);
65
66 for (int x=0; x< 7; ++x) {
67@@ -5391,6 +5391,88 @@
68 return TEST_SUCCESS;
69 }
70
71+
72+
73+
74+/*
75+ * Test that ensures mget_execute does not end into recursive calls that finally fails
76+ */
77+static test_return_t regression_bug_490486(memcached_st *memc)
78+{
79+
80+ memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL,1);
81+ memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_NO_BLOCK,1);
82+ memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_POLL_TIMEOUT,1000);
83+ memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_SERVER_FAILURE_LIMIT,1);
84+ memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_RETRY_TIMEOUT,3600);
85+
86+ /*
87+ * I only want to hit _one_ server so I know the number of requests I'm
88+ * sending in the pipeline.
89+ */
90+ uint32_t number_of_hosts= memc->number_of_hosts;
91+ memc->number_of_hosts= 1;
92+ int max_keys= 20480;
93+
94+
95+ char **keys= calloc((size_t)max_keys, sizeof(char*));
96+ size_t *key_length=calloc((size_t)max_keys, sizeof(size_t));
97+
98+ /* First add all of the items.. */
99+ char blob[1024] = {0};
100+ memcached_return rc;
101+ for (int x= 0; x < max_keys; ++x)
102+ {
103+ char k[251];
104+ key_length[x]= (size_t)snprintf(k, sizeof(k), "0200%u", x);
105+ keys[x]= strdup(k);
106+ assert(keys[x] != NULL);
107+ rc= memcached_set(memc, keys[x], key_length[x], blob, sizeof(blob), 0, 0);
108+ assert(rc == MEMCACHED_SUCCESS || rc == MEMCACHED_BUFFERED);
109+ }
110+
111+ /* Try to get all of them with a large multiget */
112+ unsigned int counter= 0;
113+ memcached_execute_function callbacks[1]= { [0]= &callback_counter };
114+ rc= memcached_mget_execute(memc, (const char**)keys, key_length,
115+ (size_t)max_keys, callbacks, &counter, 1);
116+
117+ assert(rc == MEMCACHED_SUCCESS);
118+ char* the_value = NULL;
119+ char the_key[MEMCACHED_MAX_KEY];
120+ size_t the_key_length;
121+ size_t the_value_length;
122+ uint32_t the_flags;
123+
124+ do {
125+ the_value = memcached_fetch(memc, the_key, &the_key_length, &the_value_length, &the_flags, &rc);
126+ if((the_value!= NULL) && (rc == MEMCACHED_SUCCESS))
127+ {
128+ ++counter;
129+ free(the_value);
130+ }
131+
132+ }while( (the_value!= NULL) && (rc == MEMCACHED_SUCCESS));
133+
134+
135+ assert(rc == MEMCACHED_END);
136+
137+ /* Verify that we got all of the items */
138+ assert(counter == (unsigned int)max_keys);
139+
140+ /* Release all allocated resources */
141+ for (int x= 0; x < max_keys; ++x)
142+ free(keys[x]);
143+ free(keys);
144+ free(key_length);
145+
146+ memc->number_of_hosts= number_of_hosts;
147+ return TEST_SUCCESS;
148+}
149+
150+
151+
152+
153 test_st udp_setup_server_tests[] ={
154 {"set_udp_behavior_test", 0, set_udp_behavior_test},
155 {"add_tcp_server_udp_client_test", 0, add_tcp_server_udp_client_test},
156@@ -5567,6 +5649,7 @@
157 {"lp:442914", 1, regression_bug_442914 },
158 {"lp:447342", 1, regression_bug_447342 },
159 {"lp:463297", 1, regression_bug_463297 },
160+ {"lp:490486", 1, regression_bug_490486 },
161 {0, 0, 0}
162 };
163

Subscribers

People subscribed via source and target branches

to all changes: