From d283f47cade6fcd1256e738057667fbcca7b06c1 Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Sat, 24 Nov 2007 16:17:59 -0800 Subject: [PATCH] Bugfix for memcached_connect() so that it will not always start up servers. --- ChangeLog | 2 ++ include/memcached.h | 1 + lib/memcached.c | 2 +- lib/memcached_connect.c | 72 +++++++++++------------------------------ lib/memcached_flush.c | 23 +++---------- lib/memcached_get.c | 5 +-- lib/memcached_hash.c | 8 ++++- lib/memcached_hosts.c | 1 + lib/memcached_result.c | 5 +++ lib/memcached_storage.c | 6 ---- lib/memcached_string.c | 26 ++++++++++++--- tests/function.c | 1 + 12 files changed, 65 insertions(+), 87 deletions(-) diff --git a/ChangeLog b/ChangeLog index a10bca6d..a77151a5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,8 @@ * Added option to memcache_behavior_set() so that poll() can be timed out. * Fixed memory leak in case of using memcached_fetch_result() where no value was returned. + * Bug fixed in memcached_connect() which would cause servers that + did not need to be enabled to be enabled (performance issue). 0.10 Tue Nov 20 23:22:31 PST 2007 * Added append binary test. diff --git a/include/memcached.h b/include/memcached.h index bef7e5d6..48790ce8 100644 --- a/include/memcached.h +++ b/include/memcached.h @@ -99,6 +99,7 @@ typedef enum { typedef enum { MEMCACHED_NOT_ALLOCATED, MEMCACHED_ALLOCATED, + MEMCACHED_USED, } memcached_allocated; struct memcached_server_st { diff --git a/lib/memcached.c b/lib/memcached.c index a484462b..e40c3db4 100644 --- a/lib/memcached.c +++ b/lib/memcached.c @@ -41,7 +41,7 @@ void memcached_free(memcached_st *ptr) if (ptr->is_allocated == MEMCACHED_ALLOCATED) free(ptr); else - memset(ptr, 0, sizeof(memcached_st)); + ptr->is_allocated= MEMCACHED_USED; } /* diff --git a/lib/memcached_connect.c b/lib/memcached_connect.c index 1a7cde16..0135fc9a 100644 --- a/lib/memcached_connect.c +++ b/lib/memcached_connect.c @@ -123,13 +123,15 @@ static memcached_return tcp_connect(memcached_st *ptr, unsigned int server_key) /* Old connection junk still is in the structure */ WATCHPOINT_ASSERT(ptr->hosts[server_key].stack_responses == 0); - if (ptr->hosts[server_key].servAddr.sin_family == 0) + if (ptr->hosts[server_key].sockaddr_inited == MEMCACHED_NOT_ALLOCATED || + (!(ptr->flags & MEM_USE_CACHE_LOOKUPS))) { memcached_return rc; rc= set_hostinfo(&ptr->hosts[server_key]); if (rc != MEMCACHED_SUCCESS) return rc; + ptr->hosts[server_key].sockaddr_inited= MEMCACHED_ALLOCATED; } /* Create the socket */ @@ -208,64 +210,26 @@ memcached_return memcached_connect(memcached_st *ptr, unsigned int server_key) return MEMCACHED_NO_SERVERS; /* We need to clean up the multi startup piece */ - if (server_key) + switch (ptr->hosts[server_key].type) { + case MEMCACHED_CONNECTION_UNKNOWN: + WATCHPOINT_ASSERT(0); + rc= MEMCACHED_NOT_SUPPORTED; + break; + case MEMCACHED_CONNECTION_UDP: + rc= udp_connect(ptr, server_key); + break; + case MEMCACHED_CONNECTION_TCP: rc= tcp_connect(ptr, server_key); - switch (ptr->hosts[server_key].type) - { - case MEMCACHED_CONNECTION_UNKNOWN: - WATCHPOINT_ASSERT(0); - rc= MEMCACHED_NOT_SUPPORTED; - break; - case MEMCACHED_CONNECTION_UDP: - rc= udp_connect(ptr, server_key); - break; - case MEMCACHED_CONNECTION_TCP: - rc= tcp_connect(ptr, server_key); - break; - case MEMCACHED_CONNECTION_UNIX_SOCKET: - rc= unix_socket_connect(ptr, server_key); - break; - } - - if (rc != MEMCACHED_SUCCESS) - WATCHPOINT_ERROR(rc); + break; + case MEMCACHED_CONNECTION_UNIX_SOCKET: + rc= unix_socket_connect(ptr, server_key); + break; } - else - { - unsigned int x; - - for (x= 0; x < ptr->number_of_hosts; x++) - { - memcached_return possible_rc; - possible_rc= MEMCACHED_NOT_SUPPORTED; /* Remove warning */ + if (rc != MEMCACHED_SUCCESS) + WATCHPOINT_ERROR(rc); - switch (ptr->hosts[x].type) - { - case MEMCACHED_CONNECTION_UNKNOWN: - WATCHPOINT_ASSERT(0); - possible_rc= MEMCACHED_NOT_SUPPORTED; - break; - case MEMCACHED_CONNECTION_UDP: - possible_rc= udp_connect(ptr, x); - break; - case MEMCACHED_CONNECTION_TCP: - possible_rc= tcp_connect(ptr, x); - break; - case MEMCACHED_CONNECTION_UNIX_SOCKET: - possible_rc= unix_socket_connect(ptr, x); - break; - } - rc= MEMCACHED_SUCCESS; - - if (possible_rc != MEMCACHED_SUCCESS) - { - WATCHPOINT_ERROR(possible_rc); - rc= MEMCACHED_SOME_ERRORS; - } - } - } LIBMEMCACHED_MEMCACHED_CONNECT_END(); return rc; diff --git a/lib/memcached_flush.c b/lib/memcached_flush.c index af2303ca..5fa5d960 100644 --- a/lib/memcached_flush.c +++ b/lib/memcached_flush.c @@ -8,13 +8,8 @@ memcached_return memcached_flush(memcached_st *ptr, time_t expiration) char buffer[MEMCACHED_DEFAULT_COMMAND_SIZE]; LIBMEMCACHED_MEMCACHED_FLUSH_START(); - rc= memcached_connect(ptr, 0); - - if (rc == MEMCACHED_NO_SERVERS) - return rc; - - if (rc != MEMCACHED_SUCCESS) - rc= MEMCACHED_SOME_ERRORS; + if (ptr->number_of_hosts == 0) + return MEMCACHED_NO_SERVERS; for (x= 0; x < ptr->number_of_hosts; x++) { @@ -25,20 +20,12 @@ memcached_return memcached_flush(memcached_st *ptr, time_t expiration) send_length= snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, "flush_all\r\n"); - if (send_length >= MEMCACHED_DEFAULT_COMMAND_SIZE) - return MEMCACHED_WRITE_FAILURE; - rc= memcached_do(ptr, x, buffer, send_length, 1); - if (rc != MEMCACHED_SUCCESS) - goto error; - - rc= memcached_response(ptr, buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, x); - if (rc != MEMCACHED_SUCCESS) - rc= MEMCACHED_SOME_ERRORS; + if (rc == MEMCACHED_SUCCESS) + (void)memcached_response(ptr, buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, x); } -error: LIBMEMCACHED_MEMCACHED_FLUSH_END(); - return rc; + return MEMCACHED_SUCCESS; } diff --git a/lib/memcached_get.c b/lib/memcached_get.c index 38f6865b..c5156439 100644 --- a/lib/memcached_get.c +++ b/lib/memcached_get.c @@ -358,6 +358,8 @@ memcached_result_st *memcached_fetch_result(memcached_st *ptr, if (result == NULL) result= memcached_result_create(ptr, NULL); + WATCHPOINT_ASSERT(result->value.is_allocated != MEMCACHED_USED); + while (ptr->cursor_server < ptr->number_of_hosts) { if (!ptr->hosts[ptr->cursor_server].cursor_active) @@ -395,11 +397,10 @@ memcached_result_st *memcached_fetch_result(memcached_st *ptr, { return result; } - } error: - memcached_result_free(result); + memcached_string_reset(&result->value); return NULL; } diff --git a/lib/memcached_hash.c b/lib/memcached_hash.c index 33c1055b..32e82e76 100644 --- a/lib/memcached_hash.c +++ b/lib/memcached_hash.c @@ -86,7 +86,13 @@ unsigned int memcached_generate_hash(memcached_st *ptr, char *key, size_t key_le return 0; } else - return hash % ptr->number_of_hosts; + { + unsigned int server_key; + + server_key= hash % ptr->number_of_hosts; + + return server_key; + } } static uint64_t internal_generate_hash(char *key, size_t key_length) diff --git a/lib/memcached_hosts.c b/lib/memcached_hosts.c index 9e9c6673..bd9e29c7 100644 --- a/lib/memcached_hosts.c +++ b/lib/memcached_hosts.c @@ -16,6 +16,7 @@ static void host_reset(memcached_server_st *host, char *new_hostname, unsigned i host->type= type; host->read_ptr= host->read_buffer; host->write_ptr= host->write_buffer; + host->sockaddr_inited= MEMCACHED_NOT_ALLOCATED; } memcached_return memcached_server_push(memcached_st *ptr, memcached_server_st *list) diff --git a/lib/memcached_result.c b/lib/memcached_result.c index 49f44b23..89de4030 100644 --- a/lib/memcached_result.c +++ b/lib/memcached_result.c @@ -34,8 +34,13 @@ memcached_result_st *memcached_result_create(memcached_st *memc, void memcached_result_free(memcached_result_st *ptr) { + if (ptr == NULL) + return; + memcached_string_free(&ptr->value); if (ptr->is_allocated == MEMCACHED_ALLOCATED) free(ptr); + else + ptr->is_allocated= MEMCACHED_USED; } diff --git a/lib/memcached_storage.c b/lib/memcached_storage.c index 7b48afee..93968ad9 100644 --- a/lib/memcached_storage.c +++ b/lib/memcached_storage.c @@ -62,12 +62,6 @@ static inline memcached_return memcached_send(memcached_st *ptr, if (key_length == 0) return MEMCACHED_NO_KEY_PROVIDED; - /* Leaving this WATCHPOINT_ASSERT in since only a library fubar could blow this */ -#ifdef NOT_DONE - if (!(ptr->flags & MEM_NO_BLOCK) && ptr->write_buffer_offset != 0) - WATCHPOINT_ASSERT(0); -#endif - if (ptr->hosts == NULL || ptr->number_of_hosts == 0) return MEMCACHED_NO_SERVERS; diff --git a/lib/memcached_string.c b/lib/memcached_string.c index 42f3c326..300f10f2 100644 --- a/lib/memcached_string.c +++ b/lib/memcached_string.c @@ -70,6 +70,8 @@ memcached_return memcached_string_append_character(memcached_string_st *string, { memcached_return rc; + WATCHPOINT_ASSERT(string->is_allocated != MEMCACHED_USED); + rc= memcached_string_check(string, 1); if (rc != MEMCACHED_SUCCESS) @@ -86,6 +88,8 @@ memcached_return memcached_string_append(memcached_string_st *string, { memcached_return rc; + WATCHPOINT_ASSERT(string->is_allocated != MEMCACHED_USED); + rc= memcached_string_check(string, length); if (rc != MEMCACHED_SUCCESS) @@ -103,6 +107,8 @@ memcached_return memcached_string_append(memcached_string_st *string, size_t memcached_string_backspace(memcached_string_st *string, size_t remove) { + WATCHPOINT_ASSERT(string->is_allocated != MEMCACHED_USED); + if (string->end - string->string > remove) { size_t difference; @@ -120,6 +126,9 @@ size_t memcached_string_backspace(memcached_string_st *string, size_t remove) char *memcached_string_c_copy(memcached_string_st *string) { char *c_ptr; + + WATCHPOINT_ASSERT(string->is_allocated != MEMCACHED_USED); + c_ptr= (char *)malloc(memcached_string_length(string) * sizeof(char)); if (!c_ptr) return NULL; @@ -131,15 +140,22 @@ char *memcached_string_c_copy(memcached_string_st *string) memcached_return memcached_string_reset(memcached_string_st *string) { + WATCHPOINT_ASSERT(string->is_allocated != MEMCACHED_USED); string->end= string->string; return MEMCACHED_SUCCESS; } -void memcached_string_free(memcached_string_st *string) +void memcached_string_free(memcached_string_st *ptr) { - if (string->string) - free(string->string); - if (string->is_allocated == MEMCACHED_ALLOCATED) - free(string); + if (ptr == NULL) + return; + + if (ptr->string) + free(ptr->string); + + if (ptr->is_allocated == MEMCACHED_ALLOCATED) + free(ptr); + else + ptr->is_allocated= MEMCACHED_USED; } diff --git a/tests/function.c b/tests/function.c index 640fbfe4..3e3b4af4 100644 --- a/tests/function.c +++ b/tests/function.c @@ -645,6 +645,7 @@ uint8_t mget_result_test(memcached_st *memc) { assert(results); } + while ((results= memcached_fetch_result(memc, &results_obj, &rc)) != NULL) assert(!results); assert(rc == MEMCACHED_NOTFOUND); -- 2.30.2