Improve on algo for disabling bad hosts.

diff --git a/libmemcached/connect.c b/libmemcached/connect.c
index f0ad282b0fb81ebf7684e62a777d5278498b599c..99238d8ddba9efa216169662375bc8d311009659 100644 (file)
--- a/libmemcached/connect.c
+++ b/libmemcached/connect.c
@@ -315,6 +315,7 @@ static memcached_return_t network_connect(memcached_server_st *ptr)
         {
           (void)close(ptr->fd);
           ptr->fd= -1;
+
           return rc;
         }
       }
@@ -323,7 +324,6 @@ static memcached_return_t network_connect(memcached_server_st *ptr)
 
       if (ptr->fd != -1)
       {
-        ptr->server_failure_counter= 0;
         return MEMCACHED_SUCCESS;
       }
       use = use->ai_next;
@@ -340,17 +340,25 @@ static memcached_return_t network_connect(memcached_server_st *ptr)
       if (gettimeofday(&next_time, NULL) == 0)
         ptr->next_retry= next_time.tv_sec + ptr->root->retry_timeout;
     }
-    ptr->server_failure_counter++;
+
     if (ptr->cached_errno == 0)
       return MEMCACHED_TIMEOUT;
 
     return MEMCACHED_ERRNO; /* The last error should be from connect() */
   }
 
-  ptr->server_failure_counter= 0;
   return MEMCACHED_SUCCESS; /* The last error should be from connect() */
 }
 
+static inline void set_last_disconnected_host(memcached_server_write_instance_st ptr)
+{
+  // const_cast
+  memcached_st *root= (memcached_st *)ptr->root;
+
+  if (root->last_disconnected_server)
+    memcached_server_free(root->last_disconnected_server);
+  root->last_disconnected_server= memcached_server_clone(NULL, ptr);
+}
 
 memcached_return_t memcached_connect(memcached_server_write_instance_st ptr)
 {
@@ -359,35 +367,36 @@ memcached_return_t memcached_connect(memcached_server_write_instance_st ptr)
 
   /* both retry_timeout and server_failure_limit must be set in order to delay retrying a server on error. */
   WATCHPOINT_ASSERT(ptr->root);
-  if (ptr->root->retry_timeout && ptr->root->server_failure_limit)
+  if (ptr->root->retry_timeout && ptr->next_retry)
   {
     struct timeval curr_time;
 
     gettimeofday(&curr_time, NULL);
 
-    /* if we've had too many consecutive errors on this server, mark it dead. */
-    if (ptr->server_failure_counter >= ptr->root->server_failure_limit)
+    // We should optimize this to remove the allocation if the server was
+    // the last server to die
+    if (ptr->next_retry > curr_time.tv_sec)
     {
-      ptr->next_retry= curr_time.tv_sec + ptr->root->retry_timeout;
-      ptr->server_failure_counter= 0;
-    }
+      set_last_disconnected_host(ptr);
 
-    if (curr_time.tv_sec < ptr->next_retry)
-    {
-      memcached_st *root= (memcached_st *)ptr->root;
-      // @todo fix this by fixing behavior to no longer make use of
-      // memcached_st
-      if (memcached_behavior_get(root, MEMCACHED_BEHAVIOR_AUTO_EJECT_HOSTS))
-      {
-        run_distribution(root);
-      }
+      return MEMCACHED_SERVER_MARKED_DEAD;
+    }
+  }
 
-      if (ptr->root->last_disconnected_server)
-        memcached_server_free(ptr->root->last_disconnected_server);
-      root->last_disconnected_server= memcached_server_clone(NULL, ptr);
+  // If we are over the counter failure, we just fail. Reject host only
+  // works if you have a set number of failures.
+  if (ptr->root->server_failure_limit && ptr->server_failure_counter >= ptr->root->server_failure_limit)
+  {
+    set_last_disconnected_host(ptr);
 
-      return MEMCACHED_SERVER_MARKED_DEAD;
+    // @todo fix this by fixing behavior to no longer make use of
+    // memcached_st
+    if (_is_auto_eject_host(ptr->root))
+    {
+      run_distribution((memcached_st *)ptr->root);
     }
+
+    return MEMCACHED_SERVER_MARKED_DEAD;
   }
 
   /* We need to clean up the multi startup piece */
@@ -409,17 +418,19 @@ memcached_return_t memcached_connect(memcached_server_write_instance_st ptr)
     WATCHPOINT_ASSERT(0);
   }
 
-  LIBMEMCACHED_MEMCACHED_CONNECT_END();
-
-  unlikely ( rc != MEMCACHED_SUCCESS)
+  if (rc == MEMCACHED_SUCCESS)
   {
-    memcached_st *root= (memcached_st *)ptr->root;
+    ptr->server_failure_counter= 0;
+    ptr->next_retry= 0;
+  }
+  else
+  {
+    ptr->server_failure_counter++;
 
-    //@todo create interface around last_discontected_server
-    if (ptr->root->last_disconnected_server)
-      memcached_server_free(ptr->root->last_disconnected_server);
-    root->last_disconnected_server= memcached_server_clone(NULL, ptr);
+    set_last_disconnected_host(ptr);
   }
 
+  LIBMEMCACHED_MEMCACHED_CONNECT_END();
+
   return rc;
 }

diff --git a/tests/mem_functions.c b/tests/mem_functions.c
index ea716a3420cf575b6bdfb1314f90948df08bdd75..160a89fff26ab0a58eab98c22dd35706cb2dd711 100644 (file)
--- a/tests/mem_functions.c
+++ b/tests/mem_functions.c
@@ -3022,7 +3022,7 @@ static test_return_t auto_eject_hosts(memcached_st *trash)
   for (size_t x= 0; x < 99; x++)
   {
     memcached_autoeject(memc);
-    uint32_t server_idx = memcached_generate_hash(memc, ketama_test_cases[x].key, strlen(ketama_test_cases[x].key));
+    uint32_t server_idx= memcached_generate_hash(memc, ketama_test_cases[x].key, strlen(ketama_test_cases[x].key));
     test_true(server_idx != 2);
   }
 
@@ -5462,6 +5462,42 @@ static test_return_t test_verbosity(memcached_st *memc)
   return TEST_SUCCESS;
 }
 
+static test_return_t test_server_failure(memcached_st *memc)
+{
+  memcached_st *local_memc;
+  memcached_server_instance_st instance= memcached_server_instance_by_position(memc, 0);
+
+  local_memc= memcached_create(NULL);
+
+  memcached_server_add(local_memc, memcached_server_name(instance), memcached_server_port(instance));
+  memcached_behavior_set(local_memc, MEMCACHED_BEHAVIOR_SERVER_FAILURE_LIMIT, 2);
+
+  uint32_t server_count= memcached_server_count(local_memc);
+
+  test_true(server_count == 1);
+
+  // Disable the server
+  instance= memcached_server_instance_by_position(local_memc, 0);
+  ((memcached_server_write_instance_st)instance)->server_failure_counter= 2;
+
+  memcached_return_t rc;
+  rc= memcached_set(local_memc, "foo", strlen("foo"),
+                    NULL, 0,
+                    (time_t)0, (uint32_t)0);
+  test_true(rc == MEMCACHED_SERVER_MARKED_DEAD);
+
+  ((memcached_server_write_instance_st)instance)->server_failure_counter= 0;
+  rc= memcached_set(local_memc, "foo", strlen("foo"),
+                    NULL, 0,
+                    (time_t)0, (uint32_t)0);
+  test_true(rc == MEMCACHED_SUCCESS);
+
+
+  memcached_free(local_memc);
+
+  return TEST_SUCCESS;
+}
+
 static test_return_t test_cull_servers(memcached_st *memc)
 {
   uint32_t count = memcached_server_count(memc);
@@ -5706,6 +5742,7 @@ test_st tests[] ={
 #endif
   {"test_get_last_disconnect", 1, (test_callback_fn)test_get_last_disconnect},
   {"verbosity", 1, (test_callback_fn)test_verbosity},
+  {"test_server_failure", 1, (test_callback_fn)test_server_failure},
   {"cull_servers", 1, (test_callback_fn)test_cull_servers},
   {0, 0, 0}
 };