From: Brian Aker Date: Mon, 28 Jan 2013 11:16:27 +0000 (-0500) Subject: Cleanup around the timeout code. X-Git-Tag: 1.0.16~3^2 X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=da856edbc3fadaa1a1113d4f3a336c8374c81e46;p=m6w6%2Flibmemcached Cleanup around the timeout code. --- diff --git a/libmemcached/connect.cc b/libmemcached/connect.cc index 14873538..2ec9975c 100644 --- a/libmemcached/connect.cc +++ b/libmemcached/connect.cc @@ -64,7 +64,7 @@ # define TCP_KEEPIDLE 0 #endif -static memcached_return_t connect_poll(org::libmemcached::Instance* server) +static memcached_return_t connect_poll(org::libmemcached::Instance* server, const int connection_error) { struct pollfd fds[1]; fds[0].fd= server->fd; @@ -75,90 +75,130 @@ static memcached_return_t connect_poll(org::libmemcached::Instance* server) if (server->root->poll_timeout == 0) { - return memcached_set_error(*server, MEMCACHED_TIMEOUT, MEMCACHED_AT); + return memcached_set_error(*server, MEMCACHED_TIMEOUT, MEMCACHED_AT, + memcached_literal_param("The time to wait for a connection to be established was set to zero, which means it will always timeout (MEMCACHED_TIMEOUT).")); } while (--loop_max) // Should only loop on cases of ERESTART or EINTR { int number_of; - if ((number_of= poll(fds, 1, server->root->connect_timeout)) <= 0) + if ((number_of= poll(fds, 1, server->root->connect_timeout)) == -1) { - if (number_of == -1) + int local_errno= get_socket_errno(); // We cache in case closesocket() modifies errno + switch (local_errno) { - int local_errno= get_socket_errno(); // We cache in case closesocket() modifies errno - switch (local_errno) - { #ifdef TARGET_OS_LINUX - case ERESTART: + case ERESTART: #endif - case EINTR: - continue; + case EINTR: + continue; - case EFAULT: - case ENOMEM: - return memcached_set_error(*server, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT); + case EFAULT: + case ENOMEM: + return memcached_set_error(*server, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT); - case EINVAL: - return memcached_set_error(*server, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, memcached_literal_param("RLIMIT_NOFILE exceeded, or if OSX the timeout value was invalid")); + case EINVAL: + return memcached_set_error(*server, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("RLIMIT_NOFILE exceeded, or if OSX the timeout value was invalid")); - default: // This should not happen - if (fds[0].revents & POLLERR) + default: // This should not happen + break; +#if 0 + if (fds[0].revents & POLLERR) + { + int err; + socklen_t len= sizeof(err); + if (getsockopt(server->fd, SOL_SOCKET, SO_ERROR, (char*)&err, &len) == 0) { - int err; - socklen_t len= sizeof(err); - if (getsockopt(server->fd, SOL_SOCKET, SO_ERROR, (char*)&err, &len) == 0) + if (err == 0) { - if (err == 0) - { - // This should never happen, if it does? Punt. - continue; - } - local_errno= err; + // This should never happen, if it does? Punt. + continue; } + local_errno= err; } + } +#endif + } - assert_msg(server->fd != INVALID_SOCKET, "poll() was passed an invalid file descriptor"); - server->reset_socket(); - server->state= MEMCACHED_SERVER_STATE_NEW; + assert_msg(server->fd != INVALID_SOCKET, "poll() was passed an invalid file descriptor"); + server->reset_socket(); + server->state= MEMCACHED_SERVER_STATE_NEW; + + return memcached_set_errno(*server, local_errno, MEMCACHED_AT); + } - return memcached_set_errno(*server, local_errno, MEMCACHED_AT); + if (number_of == 0) + { + if (connection_error == EINPROGRESS) + { + int err; + socklen_t len= sizeof(err); + if (getsockopt(server->fd, SOL_SOCKET, SO_ERROR, (char*)&err, &len) == -1) + { + return memcached_set_errno(*server, errno, MEMCACHED_AT, memcached_literal_param("getsockopt() error'ed while looking for error connect_poll(EINPROGRESS)")); + } + + // If Zero, my hero, we just fail to a generic MEMCACHED_TIMEOUT error + if (err != 0) + { + return memcached_set_errno(*server, err, MEMCACHED_AT, memcached_literal_param("getsockopt() found the error from poll() after connect() returned EINPROGRESS.")); } } - assert(number_of == 0); - server->io_wait_count.timeouts++; - return memcached_set_error(*server, MEMCACHED_TIMEOUT, MEMCACHED_AT); + return memcached_set_error(*server, MEMCACHED_TIMEOUT, MEMCACHED_AT); } #if 0 server->revents(fds[0].revents); #endif + assert (number_of == 1); + if (fds[0].revents & POLLERR or fds[0].revents & POLLHUP or fds[0].revents & POLLNVAL) { int err; socklen_t len= sizeof (err); - if (getsockopt(fds[0].fd, SOL_SOCKET, SO_ERROR, (char*)&err, &len) == 0) + if (getsockopt(fds[0].fd, SOL_SOCKET, SO_ERROR, (char*)&err, &len) == -1) { - // We check the value to see what happened wth the socket. - if (err == 0) - { - return MEMCACHED_SUCCESS; - } - errno= err; + return memcached_set_errno(*server, errno, MEMCACHED_AT, memcached_literal_param("getsockopt() errored while looking up error state from poll()")); } - return memcached_set_errno(*server, err, MEMCACHED_AT); + // We check the value to see what happened wth the socket. + if (err == 0) // Should not happen + { + return MEMCACHED_SUCCESS; + } + errno= err; + + return memcached_set_errno(*server, err, MEMCACHED_AT, memcached_literal_param("getsockopt() found the error from poll() during connect.")); } - assert(fds[0].revents & POLLIN or fds[0].revents & POLLOUT); + assert(fds[0].revents & POLLOUT); - return MEMCACHED_SUCCESS; + if (fds[0].revents & POLLOUT and connection_error == EINPROGRESS) + { + int err; + socklen_t len= sizeof(err); + if (getsockopt(server->fd, SOL_SOCKET, SO_ERROR, (char*)&err, &len) == -1) + { + return memcached_set_errno(*server, errno, MEMCACHED_AT); + } + + if (err == 0) + { + return MEMCACHED_SUCCESS; + } + + return memcached_set_errno(*server, err, MEMCACHED_AT, memcached_literal_param("getsockopt() found the error from poll() after connect() returned EINPROGRESS.")); + } + + break; // We only have the loop setup for errno types that require restart } // This should only be possible from ERESTART or EINTR; - return memcached_set_errno(*server, get_socket_errno(), MEMCACHED_AT); + return memcached_set_errno(*server, connection_error, MEMCACHED_AT, memcached_literal_param("connect_poll() was exhausted")); } static memcached_return_t set_hostinfo(org::libmemcached::Instance* server) @@ -554,7 +594,8 @@ static memcached_return_t network_connect(org::libmemcached::Instance* server) } /* An error occurred */ - switch (get_socket_errno()) + int local_error= get_socket_errno(); + switch (local_error) { case ETIMEDOUT: timeout_error_occured= true; @@ -569,7 +610,7 @@ static memcached_return_t network_connect(org::libmemcached::Instance* server) { server->events(POLLOUT); server->state= MEMCACHED_SERVER_STATE_IN_PROGRESS; - memcached_return_t rc= connect_poll(server); + memcached_return_t rc= connect_poll(server, local_error); if (memcached_success(rc)) { diff --git a/libmemcached/error.cc b/libmemcached/error.cc index d2380254..314f3623 100644 --- a/libmemcached/error.cc +++ b/libmemcached/error.cc @@ -59,20 +59,21 @@ static void _set(org::libmemcached::Instance& server, Memcached& memc) memcached_error_free(server); } - if (memc.error_messages == NULL) + if (memc.error_messages) { - return; - } + if (memc.error_messages->rc == MEMCACHED_TIMEOUT) + { + server.io_wait_count.timeouts++; + } - memcached_error_t *error= libmemcached_xmalloc(&memc, memcached_error_t); - if (error == NULL) // Bad business if this happens - { - return; + memcached_error_t *error= libmemcached_xmalloc(&memc, memcached_error_t); + if (error) + { + memcpy(error, memc.error_messages, sizeof(memcached_error_t)); + error->next= server.error_messages; + server.error_messages= error; + } } - - memcpy(error, memc.error_messages, sizeof(memcached_error_t)); - error->next= server.error_messages; - server.error_messages= error; } #if 0 diff --git a/libmemcached/get.cc b/libmemcached/get.cc index 2a25d788..1de5f76f 100644 --- a/libmemcached/get.cc +++ b/libmemcached/get.cc @@ -57,7 +57,6 @@ static memcached_return_t memcached_mget_by_key_real(memcached_st *ptr, const size_t *key_length, size_t number_of_keys, bool mget_mode); - char *memcached_get_by_key(memcached_st *shell, const char *group_key, size_t group_key_length, diff --git a/libmemcached/io.cc b/libmemcached/io.cc index 0660fe9d..f6cc3696 100644 --- a/libmemcached/io.cc +++ b/libmemcached/io.cc @@ -212,7 +212,6 @@ static memcached_return_t io_wait(org::libmemcached::Instance* instance, if (instance->root->poll_timeout == 0) // Mimic 0 causes timeout behavior (not all platforms do this) { - instance->io_wait_count.timeouts++; return memcached_set_error(*instance, MEMCACHED_TIMEOUT, MEMCACHED_AT); } @@ -258,7 +257,6 @@ static memcached_return_t io_wait(org::libmemcached::Instance* instance, if (active_fd == 0) { - instance->io_wait_count.timeouts++; return memcached_set_error(*instance, MEMCACHED_TIMEOUT, MEMCACHED_AT); } diff --git a/tests/include.am b/tests/include.am index 0ac1d898..44fdbfa3 100644 --- a/tests/include.am +++ b/tests/include.am @@ -58,7 +58,6 @@ test-failure: tests/failure gdb-failure: tests/failure @$(GDB_COMMAND) tests/failure - tests_testhashkit_SOURCES= tests/hashkit_functions.cc tests_testhashkit_LDADD= libtest/libtest.la libhashkit/libhashkit.la $(TESTS_LDADDS) check_PROGRAMS+= tests/testhashkit diff --git a/tests/libmemcached-1.0/mem_functions.cc b/tests/libmemcached-1.0/mem_functions.cc index 44143983..46f940d9 100644 --- a/tests/libmemcached-1.0/mem_functions.cc +++ b/tests/libmemcached-1.0/mem_functions.cc @@ -4603,7 +4603,7 @@ test_return_t regression_bug_583031(memcached_st *) test_true(memc); test_compare(MEMCACHED_SUCCESS, memcached_server_add(memc, "10.2.3.4", 11211)); - memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_CONNECT_TIMEOUT, 1000); + memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_CONNECT_TIMEOUT, 3000); memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_RETRY_TIMEOUT, 1000); memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_SND_TIMEOUT, 1000); memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_RCV_TIMEOUT, 1000); @@ -4618,7 +4618,7 @@ test_return_t regression_bug_583031(memcached_st *) test_false(value); test_zero(length); - test_compare_got(MEMCACHED_TIMEOUT, rc, memcached_last_error_message(memc)); + test_compare_got(MEMCACHED_TIMEOUT, rc, memcached_error(memc)); memcached_free(memc);