From: Trond Norbye Date: Thu, 29 Oct 2009 15:36:27 +0000 (+0100) Subject: Bug 463297: Deferred deletes doesn't work on a 1.4.x server X-Git-Tag: 0.35~4^2 X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=efba39fb875baad5397ca1aa35961d3c5d5a5d20;p=m6w6%2Flibmemcached Bug 463297: Deferred deletes doesn't work on a 1.4.x server --- diff --git a/docs/memcached_delete.pod b/docs/memcached_delete.pod index 6295bf67..4a4c76e5 100644 --- a/docs/memcached_delete.pod +++ b/docs/memcached_delete.pod @@ -24,8 +24,12 @@ C Client Library for memcached (libmemcached, -lmemcached) =head1 DESCRIPTION memcached_delete() is used to delete a particular key. An expiration value -can be applied so that the key is deleted after that many seconds. -memcached_delete_by_key() works the same, but it takes a master key to +can be applied so that the key is deleted after that many seconds, but this +will only work if you are using a 1.2.x server. If you are using a 1.2.x +server and you want to use the expiration value, please call memcached_version +one time for the memcached_st instance. If you don't do this no-reply and +buffered mode will be disabled to avoid getting out of sync with the server. +memcached_delete_by_key() works the same, but it takes a master key to find the given value. =head1 RETURN diff --git a/libmemcached/memcached_constants.h b/libmemcached/memcached_constants.h index 1705bb6e..c1e0179e 100644 --- a/libmemcached/memcached_constants.h +++ b/libmemcached/memcached_constants.h @@ -17,7 +17,7 @@ #define MEMCACHED_MAX_HOST_SORT_LENGTH 86 /* Used for Ketama */ #define MEMCACHED_POINTS_PER_SERVER 100 #define MEMCACHED_POINTS_PER_SERVER_KETAMA 160 -#define MEMCACHED_CONTINUUM_SIZE MEMCACHED_POINTS_PER_SERVER*100 /* This would then set max hosts to 100 */ +#define MEMCACHED_CONTINUUM_SIZE MEMCACHED_POINTS_PER_SERVER*100 /* This would then set max hosts to 100 */ #define MEMCACHED_STRIDE 4 #define MEMCACHED_DEFAULT_TIMEOUT 1000 #define MEMCACHED_CONTINUUM_ADDITION 10 /* How many extra slots we should build for in the continuum */ @@ -63,6 +63,7 @@ typedef enum { MEMCACHED_SERVER_MARKED_DEAD, MEMCACHED_UNKNOWN_STAT_KEY, MEMCACHED_E2BIG, + MEMCACHED_INVALID_ARGUMENTS, MEMCACHED_MAXIMUM_RETURN /* Always add new error code before */ } memcached_return; diff --git a/libmemcached/memcached_delete.c b/libmemcached/memcached_delete.c index 3fc7c037..bba3c196 100644 --- a/libmemcached/memcached_delete.c +++ b/libmemcached/memcached_delete.c @@ -8,13 +8,13 @@ memcached_return memcached_delete(memcached_st *ptr, const char *key, size_t key key, key_length, expiration); } -static inline memcached_return binary_delete(memcached_st *ptr, +static inline memcached_return binary_delete(memcached_st *ptr, unsigned int server_key, - const char *key, + const char *key, size_t key_length, uint8_t flush); -memcached_return memcached_delete_by_key(memcached_st *ptr, +memcached_return memcached_delete_by_key(memcached_st *ptr, const char *master_key, size_t master_key_length, const char *key, size_t key_length, time_t expiration) @@ -27,7 +27,7 @@ memcached_return memcached_delete_by_key(memcached_st *ptr, LIBMEMCACHED_MEMCACHED_DELETE_START(); - rc= memcached_validate_key_length(key_length, + rc= memcached_validate_key_length(key_length, ptr->flags & MEM_BINARY_PROTOCOL); unlikely (rc != MEMCACHED_SUCCESS) return rc; @@ -38,24 +38,56 @@ memcached_return memcached_delete_by_key(memcached_st *ptr, server_key= memcached_generate_hash(ptr, master_key, master_key_length); to_write= (uint8_t)((ptr->flags & MEM_BUFFER_REQUESTS) ? 0 : 1); bool no_reply= (ptr->flags & MEM_NOREPLY); - - if (ptr->flags & MEM_BINARY_PROTOCOL) - rc= binary_delete(ptr, server_key, key, key_length, to_write); - else + + if (ptr->flags & MEM_BINARY_PROTOCOL) { - if (expiration) - send_length= (size_t) snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, - "delete %s%.*s %u%s\r\n", - ptr->prefix_key, - (int) key_length, key, - (uint32_t)expiration, no_reply ? " noreply" :"" ); + likely (!expiration) + rc= binary_delete(ptr, server_key, key, key_length, to_write); else - send_length= (size_t) snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, + rc= MEMCACHED_INVALID_ARGUMENTS; + } + else + { + unlikely (expiration) + { + if ((ptr->hosts[server_key].major_version == 1 && + ptr->hosts[server_key].minor_version > 2) || + ptr->hosts[server_key].major_version > 1) + { + rc= MEMCACHED_INVALID_ARGUMENTS; + goto error; + } + else + { + if (ptr->hosts[server_key].minor_version == 0) + { + if (no_reply || !to_write) + { + /* We might get out of sync with the server if we + * send this command to a server newer than 1.2.x.. + * disable no_reply and buffered mode. + */ + to_write= 1; + if (no_reply) + memcached_server_response_increment(&ptr->hosts[server_key]); + no_reply= false; + } + } + send_length= (size_t) snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, + "delete %s%.*s %u%s\r\n", + ptr->prefix_key, + (int) key_length, key, + (uint32_t)expiration, + no_reply ? " noreply" :"" ); + } + } + else + send_length= (size_t) snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, "delete %s%.*s%s\r\n", ptr->prefix_key, (int)key_length, key, no_reply ? " noreply" :""); - - if (send_length >= MEMCACHED_DEFAULT_COMMAND_SIZE) + + if (send_length >= MEMCACHED_DEFAULT_COMMAND_SIZE) { rc= MEMCACHED_WRITE_FAILURE; goto error; @@ -68,14 +100,14 @@ memcached_return memcached_delete_by_key(memcached_st *ptr, if (send_length + ptr->hosts[server_key].write_buffer_offset > MAX_UDP_DATAGRAM_LENGTH) memcached_io_write(&ptr->hosts[server_key], NULL, 0, 1); } - + rc= memcached_do(&ptr->hosts[server_key], buffer, send_length, to_write); } if (rc != MEMCACHED_SUCCESS) goto error; - if ((ptr->flags & MEM_BUFFER_REQUESTS)) + if (!to_write) rc= MEMCACHED_BUFFERED; else if (!no_reply) { @@ -92,9 +124,9 @@ error: return rc; } -static inline memcached_return binary_delete(memcached_st *ptr, +static inline memcached_return binary_delete(memcached_st *ptr, unsigned int server_key, - const char *key, + const char *key, size_t key_length, uint8_t flush) { @@ -117,19 +149,19 @@ static inline memcached_return binary_delete(memcached_st *ptr, if (cmd_size + ptr->hosts[server_key].write_buffer_offset > MAX_UDP_DATAGRAM_LENGTH) memcached_io_write(&ptr->hosts[server_key], NULL, 0, 1); } - + memcached_return rc= MEMCACHED_SUCCESS; - if ((memcached_do(&ptr->hosts[server_key], request.bytes, + if ((memcached_do(&ptr->hosts[server_key], request.bytes, sizeof(request.bytes), 0) != MEMCACHED_SUCCESS) || - (memcached_io_write(&ptr->hosts[server_key], key, - key_length, (char) flush) == -1)) + (memcached_io_write(&ptr->hosts[server_key], key, + key_length, (char) flush) == -1)) { memcached_io_reset(&ptr->hosts[server_key]); rc= MEMCACHED_WRITE_FAILURE; } - unlikely (ptr->number_of_replicas > 0) + unlikely (ptr->number_of_replicas > 0) { request.message.header.request.opcode= PROTOCOL_BINARY_CMD_DELETEQ; @@ -138,9 +170,9 @@ static inline memcached_return binary_delete(memcached_st *ptr, ++server_key; if (server_key == ptr->number_of_hosts) server_key= 0; - + memcached_server_st* server= &ptr->hosts[server_key]; - if ((memcached_do(server, (const char*)request.bytes, + if ((memcached_do(server, (const char*)request.bytes, sizeof(request.bytes), 0) != MEMCACHED_SUCCESS) || (memcached_io_write(server, key, key_length, (char) flush) == -1)) memcached_io_reset(server); diff --git a/libmemcached/memcached_strerror.c b/libmemcached/memcached_strerror.c index 5a8b3695..28f55a21 100644 --- a/libmemcached/memcached_strerror.c +++ b/libmemcached/memcached_strerror.c @@ -80,6 +80,8 @@ const char *memcached_strerror(memcached_st *ptr __attribute__((unused)), memcac return "ENCOUNTERED AN UNKNOWN STAT KEY"; case MEMCACHED_E2BIG: return "ITEM TOO BIG"; + case MEMCACHED_INVALID_ARGUMENTS: + return "INVALID ARGUMENTS"; case MEMCACHED_MAXIMUM_RETURN: return "Gibberish returned!"; default: diff --git a/tests/function.c b/tests/function.c index 2bc0192b..648f58f7 100644 --- a/tests/function.c +++ b/tests/function.c @@ -297,10 +297,10 @@ static test_return_t error_test(memcached_st *memc) 4269430871U, 610793021U, 527273862U, 1437122909U, 2300930706U, 2943759320U, 674306647U, 2400528935U, 54481931U, 4186304426U, 1741088401U, 2979625118U, - 4159057246U, 3425930182U}; + 4159057246U, 3425930182U, 2593724503U}; // You have updated the memcache_error messages but not updated docs/tests. - assert(MEMCACHED_MAXIMUM_RETURN == 38); + assert(MEMCACHED_MAXIMUM_RETURN == 39); for (rc= MEMCACHED_SUCCESS; rc < MEMCACHED_MAXIMUM_RETURN; rc++) { uint32_t hash_val; @@ -1463,7 +1463,7 @@ static test_return_t mget_execute(memcached_st *memc) memc->number_of_hosts= 1; int max_keys= binary ? 20480 : 1; - + char **keys= calloc((size_t)max_keys, sizeof(char*)); size_t *key_length=calloc((size_t)max_keys, sizeof(size_t)); @@ -4834,8 +4834,8 @@ static test_return_t regression_bug_447342(memcached_st *memc) ** to do this ;-) */ memcached_quit(memc); - - /* Verify that all messages are stored, and we didn't stuff too much + + /* Verify that all messages are stored, and we didn't stuff too much * into the servers */ rc= memcached_mget(memc, (const char* const *)keys, key_length, max_keys); @@ -4904,6 +4904,63 @@ static test_return_t regression_bug_447342(memcached_st *memc) return TEST_SUCCESS; } +static test_return_t regression_bug_463297(memcached_st *memc) +{ + memcached_st *memc_clone= memcached_clone(NULL, memc); + assert(memc_clone != NULL); + assert(memcached_version(memc_clone) == MEMCACHED_SUCCESS); + + if (memc_clone->hosts[0].major_version > 1 || + (memc_clone->hosts[0].major_version == 1 && + memc_clone->hosts[0].minor_version > 2)) + { + /* Binary protocol doesn't support deferred delete */ + memcached_st *bin_clone= memcached_clone(NULL, memc); + assert(bin_clone != NULL); + assert(memcached_behavior_set(bin_clone, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL, 1) == MEMCACHED_SUCCESS); + assert(memcached_delete(bin_clone, "foo", 3, 1) == MEMCACHED_INVALID_ARGUMENTS); + memcached_free(bin_clone); + + memcached_quit(memc_clone); + + /* If we know the server version, deferred delete should fail + * with invalid arguments */ + assert(memcached_delete(memc_clone, "foo", 3, 1) == MEMCACHED_INVALID_ARGUMENTS); + + /* If we don't know the server version, we should get a protocol error */ + memcached_return rc= memcached_delete(memc, "foo", 3, 1); + /* but there is a bug in some of the memcached servers (1.4) that treats + * the counter as noreply so it doesn't send the proper error message + */ + assert(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR); + + /* And buffered mode should be disabled and we should get protocol error */ + assert(memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_BUFFER_REQUESTS, 1) == MEMCACHED_SUCCESS); + rc= memcached_delete(memc, "foo", 3, 1); + assert(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR); + + /* Same goes for noreply... */ + assert(memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_NOREPLY, 1) == MEMCACHED_SUCCESS); + rc= memcached_delete(memc, "foo", 3, 1); + assert(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR); + + /* but a normal request should go through (and be buffered) */ + assert((rc= memcached_delete(memc, "foo", 3, 0)) == MEMCACHED_BUFFERED); + assert(memcached_flush_buffers(memc) == MEMCACHED_SUCCESS); + + assert(memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_BUFFER_REQUESTS, 0) == MEMCACHED_SUCCESS); + /* unbuffered noreply should be success */ + assert(memcached_delete(memc, "foo", 3, 0) == MEMCACHED_SUCCESS); + /* unbuffered with reply should be not found... */ + assert(memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_NOREPLY, 0) == MEMCACHED_SUCCESS); + assert(memcached_delete(memc, "foo", 3, 0) == MEMCACHED_NOTFOUND); + } + + memcached_free(memc_clone); + return TEST_SUCCESS; +} + + /* Test memcached_server_get_last_disconnect * For a working server set, shall be NULL * For a set of non existing server, shall not be NULL @@ -5172,6 +5229,7 @@ test_st regression_tests[]= { {"lp:421108", 1, regression_bug_421108 }, {"lp:442914", 1, regression_bug_442914 }, {"lp:447342", 1, regression_bug_447342 }, + {"lp:463297", 1, regression_bug_463297 }, {0, 0, 0} };