Extend pool API for fetch/release. Fix concurrency issue in test case.
authorBrian Aker <brian@tangent.org>
Mon, 26 Sep 2011 06:13:04 +0000 (23:13 -0700)
committerBrian Aker <brian@tangent.org>
Mon, 26 Sep 2011 06:13:04 +0000 (23:13 -0700)
docs/conf.py.in
docs/include.am
docs/memcached_pool.rst
libmemcached/error.cc
libmemcached/util/pool.cc
libmemcached/util/pool.h
support/libmemcached.spec.in
tests/mem_functions.cc

index c485d77cee03b2debd7cae5a3fc69fd22eceb070..5952dc2ea7e36b0e6253c4d44648f1b2e6514d98 100644 (file)
@@ -290,9 +290,10 @@ man_pages = [
   ('memcached_pool', 'memcached_pool_behavior_set', u'libmemcached Documentation', [u'Brian Aker'], 3),
   ('memcached_pool', 'memcached_pool_create', u'libmemcached Documentation', [u'Brian Aker'], 3),
   ('memcached_pool', 'memcached_pool_destroy', u'libmemcached Documentation', [u'Brian Aker'], 3),
+  ('memcached_pool', 'memcached_pool_fetch', u'libmemcached Documentation', [u'Brian Aker'], 3),
   ('memcached_pool', 'memcached_pool_pop', u'libmemcached Documentation', [u'Brian Aker'], 3),
   ('memcached_pool', 'memcached_pool_push', u'libmemcached Documentation', [u'Brian Aker'], 3),
-  ('memcached_pool', 'memcached_pool_push', u'libmemcached Documentation', [u'Brian Aker'], 3),
+  ('memcached_pool', 'memcached_pool_release', u'libmemcached Documentation', [u'Brian Aker'], 3),
   ('memcached_pool', 'memcached_pool_st', u'libmemcached Documentation', [u'Brian Aker'], 3),
   ('memcached_quit', 'memcached_quit', u'libmemcached Documentation', [u'Brian Aker'], 3),
   ('memcached_result_st', 'memcached_result_cas', u'Working with result sets', [u'Brian Aker'], 3),
index 1a1feac04bd318ecb9b36bed1850b89ec63f9fd1..51b9ed73d72f267bf34648209e45a127060f334f 100644 (file)
@@ -80,12 +80,15 @@ man_MANS+= \
           docs/man/memcached_mget_by_key.3 \
           docs/man/memcached_mget_execute.3  \
           docs/man/memcached_mget_execute_by_key.3  \
+          docs/man/memcached_pool.3 \
           docs/man/memcached_pool_behavior_get.3 \
           docs/man/memcached_pool_behavior_set.3 \
           docs/man/memcached_pool_create.3 \
           docs/man/memcached_pool_destroy.3 \
+          docs/man/memcached_pool_fetch.3 \
           docs/man/memcached_pool_pop.3 \
           docs/man/memcached_pool_push.3 \
+          docs/man/memcached_pool_release.3 \
           docs/man/memcached_pool_st.3 \
           docs/man/memcached_prepend.3 \
           docs/man/memcached_prepend_by_key.3 \
index 7dcc707a9c081152d9f0868fd4400524abb71df4..3f8882f83661ab51d7fd689933717d58552ee9f8 100644 (file)
@@ -14,13 +14,26 @@ SYNOPSIS
 
 .. c:function:: memcached_pool_st* memcached_pool_create(memcached_st* mmc, int initial, int max)
 .. deprecated:: 0.46
-   Use :c:func:`memcached_pool()` instead.
+   Use :c:func:`memcached_pool()`
  
 .. c:function:: memcached_st* memcached_pool_destroy(memcached_pool_st* pool)
  
-.. c:function:: memcached_st* memcached_pool_pop (memcached_pool_st* pool, bool block, memcached_return_t *rc)
+.. c:function:: memcached_st* memcached_pool_pop(memcached_pool_st* pool, bool block, memcached_return_t *rc)
+.. deprecated:: 0.53
+   Use :c:func:`memcached_pool_fetch()`
+
+.. c:function:: memcached_st* memcached_pool_fetch(memcached_pool_st*, struct timespec* relative_time, memcached_return_t* rc);
+
+.. versionadded:: 0.53
+   Synonym for memcached_pool_pop()
+
 .. c:function:: memcached_return_t memcached_pool_push(memcached_pool_st* pool, memcached_st *mmc)
+.. deprecated:: 0.53
+   Use :c:func:`memcached_pool_push()`
+
+.. c:function:: memcached_return_t memcached_pool_release(memcached_pool_st* pool, memcached_st* mmc);
+.. versionadded:: 0.53
+   Synonym for memcached_pool_push().
  
 .. c:function:: memcached_return_t memcached_pool_behavior_set(memcached_pool_st *pool, memcached_behavior_t flag, uint64_t data)
  
@@ -28,7 +41,6 @@ SYNOPSIS
 
 Compile and link with -lmemcachedutil -lmemcached
 
-
 -----------
 DESCRIPTION
 -----------
@@ -43,15 +55,16 @@ created with :c:func:`memcached_pool_create()` and release all allocated
 resources. It will return the pointer to the :c:type:`memcached_st` structure
 passed as an argument to :c:func:`memcached_pool_create()`, and returns the ownership of the pointer to the caller when created with :c:func:`memcached_pool_create()`, otherwise NULL is returned..
 
-:c:func:`memcached_pool_pop()` is used to grab a connection structure from the
-connection pool. The block argument specifies if the function should
+:c:func:`memcached_pool_fetch()` is used to fetch a connection structure from the
+connection pool. The relative_time argument specifies if the function should
 block and wait for a connection structure to be available if we try
-to exceed the maximum size.
+to exceed the maximum size. You need to specify time in relative time.
 
-:c:func:`memcached_pool_push()` is used to return a connection structure back to the pool.
+:c:func:`memcached_pool_release()` is used to return a connection structure back to the pool.
 
 :c:func:`memcached_pool_behavior_get()` and :c:func:`memcached_pool_behavior_set()` is used to get/set behavior flags on all connections in the pool.
 
+Both :c:func:`memcached_pool_release()` and :c:func:`memcached_pool_fetch()` are thread safe.
 
 ------
 RETURN
@@ -61,10 +74,14 @@ RETURN
 
 :c:func:`memcached_pool_pop()` returns a pointer to a :c:type:`memcached_st` structure from the pool (or NULL if an allocation cannot be satisfied).
 
-:c:func:`memcached_pool_push()` returns :c:type:`MEMCACHED_SUCCESS` upon success.
+:c:func:`memcached_pool_release()` returns :c:type:`MEMCACHED_SUCCESS` upon success.
 
 :c:func:`memcached_pool_behavior_get()` and :c:func:`memcached_pool_behavior_get()` returns :c:type:`MEMCACHED_SUCCESS` upon success.
 
+If any methods returns MEMCACHED_IN_PROGRESS then a lock on the pool could not be obtained. If any of the parameters passed to any of these functions is invalid, MEMCACHED_INVALID_ARGUMENTS will be returned.
+
+memcached_pool_fetch() may return MEMCACHED_TIMEOUT if a timeout occurs while waiting for a free memcached_st. MEMCACHED_NOTFOUND if no memcached_st was available.
+
 
 ----
 HOME
index afb606451fefcfba19b89bc21743f11996cf7ef4..d6b7f16613734bdea1e86c59a0339a8247eb8c2e 100644 (file)
@@ -361,10 +361,12 @@ memcached_return_t memcached_set_errno(memcached_server_st& self, int local_errn
 
 static void _error_print(const memcached_error_t *error)
 {
-  if (not error)
+  if (error == NULL)
+  {
     return;
+  }
 
-  if (not error->size)
+  if (error->size == 0)
   {
     fprintf(stderr, "%s\n", memcached_strerror(NULL, error->rc) );
   }
index e89789d4aadccc63eba79816249f497c8700b963..47f46eec224b7d726ecc4fd3c9e446cbb04221e5 100644 (file)
 #include <libmemcached/common.h>
 #include <libmemcached/memcached_util.h>
 
+#include <libmemcached/error.hpp>
+
 #include <cassert>
 #include <cerrno>
 #include <pthread.h>
 #include <memory>
 
-static bool grow_pool(memcached_pool_st* pool);
-
 struct memcached_pool_st
 {
   pthread_mutex_t mutex;
@@ -56,6 +56,7 @@ struct memcached_pool_st
   const uint32_t size;
   uint32_t current_size;
   bool _owns_master;
+  struct timespec _timeout;
 
   memcached_pool_st(memcached_st *master_arg, size_t max_arg) :
     master(master_arg),
@@ -67,26 +68,21 @@ struct memcached_pool_st
   {
     pthread_mutex_init(&mutex, NULL);
     pthread_cond_init(&cond, NULL);
+    _timeout.tv_sec= 5;
+    _timeout.tv_nsec= 0;
   }
 
-  bool init(uint32_t initial)
+  const struct timespec& timeout() const
   {
-    server_pool= new (std::nothrow) memcached_st *[size];
-    if (not server_pool)
-      return false;
+    return _timeout;
+  }
 
-    /*
-      Try to create the initial size of the pool. An allocation failure at
-      this time is not fatal..
-    */
-    for (unsigned int x= 0; x < initial; ++x)
-    {
-      if (not grow_pool(this))
-        break;
-    }
+  bool release(memcached_st*, memcached_return_t& rc);
 
-    return true;
-  }
+  memcached_st *fetch(memcached_return_t& rc);
+  memcached_st *fetch(const struct timespec&, memcached_return_t& rc);
+
+  bool init(uint32_t initial);
 
   ~memcached_pool_st()
   {
@@ -121,27 +117,6 @@ struct memcached_pool_st
   }
 };
 
-static memcached_return_t mutex_enter(pthread_mutex_t *mutex)
-{
-  int ret;
-  do
-  {
-    ret= pthread_mutex_lock(mutex);
-  } while (ret == -1 && errno == EINTR);
-
-  return (ret == -1) ? MEMCACHED_ERRNO : MEMCACHED_SUCCESS;
-}
-
-static memcached_return_t mutex_exit(pthread_mutex_t *mutex)
-{
-  int ret;
-  do
-  {
-    ret= pthread_mutex_unlock(mutex);
-  } while (ret == -1 && errno == EINTR);
-
-  return (ret == -1) ? MEMCACHED_ERRNO : MEMCACHED_SUCCESS;
-}
 
 /**
  * Grow the connection pool by creating a connection structure and clone the
@@ -149,6 +124,8 @@ static memcached_return_t mutex_exit(pthread_mutex_t *mutex)
  */
 static bool grow_pool(memcached_pool_st* pool)
 {
+  assert(pool);
+
   memcached_st *obj;
   if (not (obj= memcached_clone(NULL, pool->master)))
   {
@@ -162,18 +139,38 @@ static bool grow_pool(memcached_pool_st* pool)
   return true;
 }
 
+bool memcached_pool_st::init(uint32_t initial)
+{
+  server_pool= new (std::nothrow) memcached_st *[size];
+  if (not server_pool)
+    return false;
+
+  /*
+    Try to create the initial size of the pool. An allocation failure at
+    this time is not fatal..
+  */
+  for (unsigned int x= 0; x < initial; ++x)
+  {
+    if (grow_pool(this) == false)
+    {
+      break;
+    }
+  }
+
+  return true;
+}
+
+
 static inline memcached_pool_st *_pool_create(memcached_st* master, uint32_t initial, uint32_t max)
 {
   if (initial == 0 or max == 0 or (initial > max))
   {
-    errno= EINVAL;
     return NULL;
   }
 
   memcached_pool_st *object= new (std::nothrow) memcached_pool_st(master, max);
   if (object == NULL)
   {
-    errno= ENOMEM; // Set this for the failed calloc
     return NULL;
   }
 
@@ -208,10 +205,8 @@ memcached_pool_st * memcached_pool(const char *option_string, size_t option_stri
   if (self == NULL)
   {
     memcached_free(memc);
-    errno= ENOMEM;
     return NULL;
   }
-  errno= 0;
 
   self->_owns_master= true;
 
@@ -239,121 +234,180 @@ memcached_st*  memcached_pool_destroy(memcached_pool_st* pool)
   return ret;
 }
 
-memcached_st* memcached_pool_pop(memcached_pool_st* pool,
-                                 bool block,
-                                 memcached_return_t *rc)
+memcached_st* memcached_pool_st::fetch(memcached_return_t& rc)
 {
-  memcached_return_t not_used;
-  if (rc == NULL)
-  {
-    rc= &not_used;
-  }
-
-  if (pool == NULL)
-  {
-    *rc= MEMCACHED_INVALID_ARGUMENTS;
-    errno= EINVAL;
+  static struct timespec relative_time= { 0, 0 };
+  return fetch(relative_time, rc);
+}
 
-    return NULL;
-  }
+memcached_st* memcached_pool_st::fetch(const struct timespec& relative_time, memcached_return_t& rc)
+{
+  rc= MEMCACHED_SUCCESS;
 
-  if (memcached_failed((*rc= mutex_enter(&pool->mutex))))
+  if (pthread_mutex_lock(&mutex))
   {
+    rc= MEMCACHED_IN_PROGRESS;
     return NULL;
   }
 
   memcached_st *ret= NULL;
   do
   {
-    if (pool->firstfree > -1)
+    if (firstfree > -1)
     {
-      ret= pool->server_pool[pool->firstfree--];
+      ret= server_pool[firstfree--];
     }
-    else if (pool->current_size == pool->size)
+    else if (current_size == size)
     {
-      if (block == false)
+      if (relative_time.tv_sec == 0 and relative_time.tv_nsec == 0)
       {
-        *rc= mutex_exit(&pool->mutex); // this should be a different error
+        pthread_mutex_unlock(&mutex);
+        rc= MEMCACHED_NOTFOUND;
+
         return NULL;
       }
 
       struct timespec time_to_wait= {0, 0};
-      time_to_wait.tv_sec = time(NULL) +5;
+      time_to_wait.tv_sec= time(NULL) +relative_time.tv_sec;
+      time_to_wait.tv_nsec= relative_time.tv_nsec;
+
       int thread_ret;
-      if ((thread_ret= pthread_cond_timedwait(&pool->cond, &pool->mutex, &time_to_wait)) != 0)
+      if ((thread_ret= pthread_cond_timedwait(&cond, &mutex, &time_to_wait)) != 0)
       {
-        mutex_exit(&pool->mutex);
-        errno= thread_ret;
+        pthread_mutex_unlock(&mutex);
 
         if (thread_ret == ETIMEDOUT)
         {
-          *rc= MEMCACHED_TIMEOUT;
+          rc= MEMCACHED_TIMEOUT;
         }
         else
         {
-          *rc= MEMCACHED_ERRNO;
+          errno= thread_ret;
+          rc= MEMCACHED_ERRNO;
         }
 
         return NULL;
       }
     }
-    else if (not grow_pool(pool))
+    else if (grow_pool(this) == false)
     {
-      (void)mutex_exit(&pool->mutex);
-      *rc= MEMCACHED_MEMORY_ALLOCATION_FAILURE;
+      (void)pthread_mutex_unlock(&mutex);
       return NULL;
     }
   } while (ret == NULL);
 
-  *rc= mutex_exit(&pool->mutex);
+  pthread_mutex_unlock(&mutex);
 
   return ret;
 }
 
-memcached_return_t memcached_pool_push(memcached_pool_st* pool, memcached_st *released)
+bool memcached_pool_st::release(memcached_st *released, memcached_return_t& rc)
 {
-  if (pool == NULL)
-  {
-    return MEMCACHED_INVALID_ARGUMENTS;
-  }
-
+  rc= MEMCACHED_SUCCESS;
   if (released == NULL)
   {
-    return MEMCACHED_INVALID_ARGUMENTS;
+    rc= MEMCACHED_INVALID_ARGUMENTS;
+    return false;
   }
 
-  memcached_return_t rc= mutex_enter(&pool->mutex);
-
-  if (rc != MEMCACHED_SUCCESS)
+  if (pthread_mutex_lock(&mutex))
   {
-    return rc;
+    rc= MEMCACHED_IN_PROGRESS;
+    return false;
   }
 
-  /* Someone updated the behavior on the object.. */
-  if (pool->compare_version(released) == false)
+  /* 
+    Someone updated the behavior on the object, so we clone a new memcached_st with the new settings. If we fail to clone, we keep the old one around.
+  */
+  if (compare_version(released) == false)
   {
-    memcached_free(released);
-    if (not (released= memcached_clone(NULL, pool->master)))
+    memcached_st *memc;
+    if ((memc= memcached_clone(NULL, master)))
     {
-      rc= MEMCACHED_SOME_ERRORS;
+      memcached_free(released);
+      released= memc;
     }
   }
 
-  pool->server_pool[++pool->firstfree]= released;
+  server_pool[++firstfree]= released;
 
-  if (pool->firstfree == 0 and pool->current_size == pool->size)
+  if (firstfree == 0 and current_size == size)
   {
     /* we might have people waiting for a connection.. wake them up :-) */
-    pthread_cond_broadcast(&pool->cond);
+    pthread_cond_broadcast(&cond);
   }
 
-  memcached_return_t rval= mutex_exit(&pool->mutex);
-  if (rc == MEMCACHED_SOME_ERRORS)
+  (void)pthread_mutex_unlock(&mutex);
+
+  return true;
+}
+
+memcached_st* memcached_pool_fetch(memcached_pool_st* pool, struct timespec* relative_time, memcached_return_t* rc)
+{
+  if (pool == NULL)
   {
-    return rc;
+    return NULL;
+  }
+
+  memcached_return_t unused;
+  if (rc == NULL)
+  {
+    rc= &unused;
   }
 
-  return rval;
+  if (relative_time == NULL)
+  {
+    return pool->fetch(*rc);
+  }
+
+  return pool->fetch(*relative_time, *rc);
+}
+
+memcached_st* memcached_pool_pop(memcached_pool_st* pool,
+                                 bool block,
+                                 memcached_return_t *rc)
+{
+  if (pool == NULL)
+  {
+    return NULL;
+  }
+
+  memcached_return_t unused;
+  if (rc == NULL)
+  {
+    rc= &unused;
+  }
+
+  memcached_st *memc;
+  if (block)
+  {
+    memc= pool->fetch(pool->timeout(), *rc);
+  }
+  else
+  {
+    memc= pool->fetch(*rc);
+  }
+
+  return memc;
+}
+
+memcached_return_t memcached_pool_release(memcached_pool_st* pool, memcached_st *released)
+{
+  if (pool == NULL)
+  {
+    return MEMCACHED_INVALID_ARGUMENTS;
+  }
+
+  memcached_return_t rc;
+
+  (void) pool->release(released, rc);
+
+  return rc;
+}
+
+memcached_return_t memcached_pool_push(memcached_pool_st* pool, memcached_st *released)
+{
+  return memcached_pool_release(pool, released);
 }
 
 
@@ -366,15 +420,16 @@ memcached_return_t memcached_pool_behavior_set(memcached_pool_st *pool,
     return MEMCACHED_INVALID_ARGUMENTS;
   }
 
-  memcached_return_t rc= mutex_enter(&pool->mutex);
-  if (rc != MEMCACHED_SUCCESS)
-    return rc;
+  if (pthread_mutex_lock(&pool->mutex))
+  {
+    return MEMCACHED_IN_PROGRESS;
+  }
 
   /* update the master */
-  rc= memcached_behavior_set(pool->master, flag, data);
-  if (rc != MEMCACHED_SUCCESS)
+  memcached_return_t rc= memcached_behavior_set(pool->master, flag, data);
+  if (memcached_failed(rc))
   {
-    mutex_exit(&pool->mutex);
+    (void)pthread_mutex_unlock(&pool->mutex);
     return rc;
   }
 
@@ -382,16 +437,17 @@ memcached_return_t memcached_pool_behavior_set(memcached_pool_st *pool,
   /* update the clones */
   for (int xx= 0; xx <= pool->firstfree; ++xx)
   {
-    rc= memcached_behavior_set(pool->server_pool[xx], flag, data);
-    if (rc == MEMCACHED_SUCCESS)
+    if (memcached_success(memcached_behavior_set(pool->server_pool[xx], flag, data)))
     {
       pool->server_pool[xx]->configure.version= pool->version();
     }
     else
     {
-      memcached_free(pool->server_pool[xx]);
-      if (not (pool->server_pool[xx]= memcached_clone(NULL, pool->master)))
+      memcached_st *memc;
+      if ((memc= memcached_clone(NULL, pool->master)))
       {
+        memcached_free(pool->server_pool[xx]);
+        pool->server_pool[xx]= memc;
         /* I'm not sure what to do in this case.. this would happen
           if we fail to push the server list inside the client..
           I should add a testcase for this, but I believe the following
@@ -402,7 +458,9 @@ memcached_return_t memcached_pool_behavior_set(memcached_pool_st *pool,
     }
   }
 
-  return mutex_exit(&pool->mutex);
+  (void)pthread_mutex_unlock(&pool->mutex);
+
+  return rc;
 }
 
 memcached_return_t memcached_pool_behavior_get(memcached_pool_st *pool,
@@ -414,13 +472,14 @@ memcached_return_t memcached_pool_behavior_get(memcached_pool_st *pool,
     return MEMCACHED_INVALID_ARGUMENTS;
   }
 
-  memcached_return_t rc= mutex_enter(&pool->mutex);
-  if (rc != MEMCACHED_SUCCESS)
+  if (pthread_mutex_lock(&pool->mutex))
   {
-    return rc;
+    return MEMCACHED_IN_PROGRESS;
   }
 
   *value= memcached_behavior_get(pool->master, flag);
 
-  return mutex_exit(&pool->mutex);
+  (void)pthread_mutex_unlock(&pool->mutex);
+
+  return MEMCACHED_SUCCESS;
 }
index e4a6ce59db9ef667322e426e66421c1fcab4dacd..eba97ec26fa98d40bce86369c829aeb49166bde3 100644 (file)
@@ -63,6 +63,11 @@ memcached_st* memcached_pool_pop(memcached_pool_st* pool,
 LIBMEMCACHED_API
 memcached_return_t memcached_pool_push(memcached_pool_st* pool,
                                        memcached_st* mmc);
+LIBMEMCACHED_API
+  memcached_return_t memcached_pool_release(memcached_pool_st* pool, memcached_st* mmc);
+
+LIBMEMCACHED_API
+memcached_st* memcached_pool_fetch(memcached_pool_st*, struct timespec* relative_time, memcached_return_t* rc);
 
 LIBMEMCACHED_API
 memcached_return_t memcached_pool_behavior_set(memcached_pool_st *ptr,
index c031b7b36592d1da2d5133175ebba162bad81bc3..64abfc3234c2538871303c6a5f021fc3cbaed252 100644 (file)
@@ -245,7 +245,10 @@ you will need to install %{name}-devel.
 %{_mandir}/man3/memcached_pool_destroy.3.gz
 %{_mandir}/man3/memcached_pool_pop.3.gz
 %{_mandir}/man3/memcached_pool_push.3.gz
+%{_mandir}/man3/memcached_pool_fetch.3.gz
+%{_mandir}/man3/memcached_pool_release.3.gz
 %{_mandir}/man3/memcached_pool_st.3.gz
+%{_mandir}/man3/memcached_pool.3.gz
 %{_mandir}/man3/memcached_prepend.3.gz
 %{_mandir}/man3/memcached_prepend_by_key.3.gz
 %{_mandir}/man3/memcached_quit.3.gz
index c6f627f7d36bb61430019339db9a21697132f829..cca23464e01ac16d9b63aa21e2c5cdef6ca3395d 100644 (file)
@@ -4192,41 +4192,6 @@ static test_return_t dump_test(memcached_st *memc)
   return TEST_SUCCESS;
 }
 
-struct test_pool_context_st {
-  volatile memcached_return_t rc;
-  memcached_pool_st* pool;
-  memcached_st* mmc;
-  sem_t lock;
-
-  test_pool_context_st(memcached_pool_st *pool_arg, memcached_st *memc_arg):
-    rc(MEMCACHED_FAILURE),
-    pool(pool_arg),
-    mmc(memc_arg)
-  {
-    sem_init(&lock, 0, 0);
-  }
-
-  ~test_pool_context_st()
-  {
-    sem_destroy(&lock);
-  }
-};
-
-static void* connection_release(void *arg)
-{
-  test_pool_context_st *resource= static_cast<test_pool_context_st *>(arg);
-  assert(resource);
-  if (resource == NULL)
-  {
-    abort();
-  }
-
-  sem_wait(&resource->lock);
-  // Release all of the memc we are holding
-  resource->rc= memcached_pool_push(resource->pool, resource->mmc);
-
-  pthread_exit(arg);
-}
 
 #define POOL_SIZE 10
 static test_return_t connection_pool_test(memcached_st *memc)
@@ -4234,55 +4199,21 @@ static test_return_t connection_pool_test(memcached_st *memc)
   memcached_pool_st* pool= memcached_pool_create(memc, 5, POOL_SIZE);
   test_true(pool);
   memcached_st *mmc[POOL_SIZE];
-  memcached_return_t rc;
 
   // Fill up our array that we will store the memc that are in the pool
   for (size_t x= 0; x < POOL_SIZE; ++x)
   {
-    mmc[x]= memcached_pool_pop(pool, false, &rc);
-    test_true(mmc[x]);
+    memcached_return_t rc;
+    mmc[x]= memcached_pool_fetch(pool, NULL, &rc);
     test_compare(MEMCACHED_SUCCESS, rc);
+    test_true(mmc[x]);
   }
 
   // All memc should be gone
-  test_null(memcached_pool_pop(pool, false, &rc));
-  test_compare(MEMCACHED_SUCCESS, rc);
-
-  /*
-    @note This comment was written to describe what was believed to be the original authors intent.
-
-    This portion of the test creates a thread that will wait until told to free a memcached_st
-    that will be grabbed by the main thread.
-
-    It is believed that this tests whether or not we are handling ownership correctly.
-  */
   {
-    pthread_t tid;
-    test_pool_context_st item(pool, mmc[9]);
-
-    test_zero(pthread_create(&tid, NULL, connection_release, &item));
-    mmc[9]= memcached_pool_pop(pool, true, &rc);
-    if (rc != MEMCACHED_SUCCESS)
-    {
-      sem_post(&item.lock);
-      pthread_join(tid, NULL);
-    }
-    else if (rc == MEMCACHED_TIMEOUT)
-    {
-      Error << "Possible timing issue with memcached_pool_pop";
-    }
-    else
-    {
-      test_compare(MEMCACHED_SUCCESS, rc);
-    }
-
-    sem_post(&item.lock);
-    pthread_join(tid, NULL);
-    if (rc == MEMCACHED_SUCCESS)
-    {
-      test_compare(MEMCACHED_SUCCESS, item.rc);
-      test_true(mmc[9]);
-    }
+    memcached_return_t rc;
+    test_null(memcached_pool_fetch(pool, NULL, &rc));
+    test_compare(MEMCACHED_NOTFOUND, rc);
   }
 
   // Release them..
@@ -4290,7 +4221,7 @@ static test_return_t connection_pool_test(memcached_st *memc)
   {
     if (mmc[x])
     {
-      test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[x]));
+      test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[x]));
     }
   }
   test_true(memcached_pool_destroy(pool) == memc);
@@ -4303,19 +4234,22 @@ static test_return_t connection_pool2_test(memcached_st *memc)
   memcached_pool_st* pool= memcached_pool_create(memc, 5, POOL_SIZE);
   test_true(pool);
   memcached_st *mmc[POOL_SIZE];
-  memcached_return_t rc;
 
   // Fill up our array that we will store the memc that are in the pool
   for (size_t x= 0; x < POOL_SIZE; ++x)
   {
-    mmc[x]= memcached_pool_pop(pool, false, &rc);
-    test_true(mmc[x]);
+    memcached_return_t rc;
+    mmc[x]= memcached_pool_fetch(pool, NULL, &rc);
     test_compare(MEMCACHED_SUCCESS, rc);
+    test_true(mmc[x]);
   }
 
   // All memc should be gone
-  test_null(memcached_pool_pop(pool, false, &rc));
-  test_compare(MEMCACHED_SUCCESS, rc);
+  {
+    memcached_return_t rc;
+    test_null(memcached_pool_fetch(pool, NULL, &rc));
+    test_compare(MEMCACHED_NOTFOUND, rc);
+  }
 
   // verify that I can do ops with all connections
   test_compare(MEMCACHED_SUCCESS,
@@ -4336,7 +4270,7 @@ static test_return_t connection_pool2_test(memcached_st *memc)
   // Release them..
   for (size_t x= 0; x < POOL_SIZE; ++x)
   {
-    test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[x]));
+    test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[x]));
   }
 
 
@@ -4344,22 +4278,137 @@ static test_return_t connection_pool2_test(memcached_st *memc)
    * of the connections in the pool. It should however be enabled
    * when I push the item into the pool
    */
-  mmc[0]= memcached_pool_pop(pool, false, &rc);
+  mmc[0]= memcached_pool_fetch(pool, NULL, NULL);
   test_true(mmc[0]);
 
   test_compare(MEMCACHED_SUCCESS,
                memcached_pool_behavior_set(pool, MEMCACHED_BEHAVIOR_IO_MSG_WATERMARK, 9999));
 
-  mmc[1]= memcached_pool_pop(pool, false, &rc);
-  test_true(mmc[1]);
+  {
+    memcached_return_t rc;
+    mmc[1]= memcached_pool_fetch(pool, NULL, &rc);
+    test_true(mmc[1]);
+    test_compare(MEMCACHED_SUCCESS, rc);
+  }
 
   test_compare(UINT64_C(9999), memcached_behavior_get(mmc[1], MEMCACHED_BEHAVIOR_IO_MSG_WATERMARK));
-  test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[1]));
-  test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[0]));
+  test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[1]));
+  test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[0]));
+
+  {
+    memcached_return_t rc;
+    mmc[0]= memcached_pool_fetch(pool, NULL, &rc);
+    test_true(mmc[0]);
+    test_compare(MEMCACHED_SUCCESS, rc);
+  }
 
-  mmc[0]= memcached_pool_pop(pool, false, &rc);
   test_compare(UINT64_C(9999), memcached_behavior_get(mmc[0], MEMCACHED_BEHAVIOR_IO_MSG_WATERMARK));
-  test_compare(MEMCACHED_SUCCESS, memcached_pool_push(pool, mmc[0]));
+  test_compare(MEMCACHED_SUCCESS, memcached_pool_release(pool, mmc[0]));
+
+  test_true(memcached_pool_destroy(pool) == memc);
+
+  return TEST_SUCCESS;
+}
+
+struct test_pool_context_st {
+  volatile memcached_return_t rc;
+  memcached_pool_st* pool;
+  memcached_st* mmc;
+  sem_t _lock;
+
+  test_pool_context_st(memcached_pool_st *pool_arg, memcached_st *memc_arg):
+    rc(MEMCACHED_FAILURE),
+    pool(pool_arg),
+    mmc(memc_arg)
+  {
+    sem_init(&_lock, 0, 0);
+  }
+
+  void wait()
+  {
+    sem_wait(&_lock);
+  }
+
+  void release()
+  {
+    sem_post(&_lock);
+  }
+
+  ~test_pool_context_st()
+  {
+    sem_destroy(&_lock);
+  }
+};
+
+static void* connection_release(void *arg)
+{
+  test_pool_context_st *resource= static_cast<test_pool_context_st *>(arg);
+  assert(resource);
+  if (resource == NULL)
+  {
+    abort();
+  }
+
+  // Release all of the memc we are holding 
+  resource->rc= memcached_pool_release(resource->pool, resource->mmc);
+  resource->release();
+
+  pthread_exit(arg);
+}
+
+static test_return_t connection_pool3_test(memcached_st *memc)
+{
+  memcached_pool_st* pool= memcached_pool_create(memc, 1, 1);
+  test_true(pool);
+
+  memcached_st *pool_memc;
+  {
+    memcached_return_t rc;
+    pool_memc= memcached_pool_fetch(pool, NULL, &rc);
+    test_compare(MEMCACHED_SUCCESS, rc);
+    test_true(pool_memc);
+  }
+
+  /*
+    @note This comment was written to describe what was believed to be the original authors intent.
+
+    This portion of the test creates a thread that will wait until told to free a memcached_st
+    that will be grabbed by the main thread.
+
+    It is believed that this tests whether or not we are handling ownership correctly.
+  */
+  pthread_t tid;
+  test_pool_context_st item(pool, pool_memc);
+
+  test_zero(pthread_create(&tid, NULL, connection_release, &item));
+  item.wait();
+
+  memcached_return_t rc;
+  memcached_st *pop_memc;
+  int counter= 5;
+  do
+  {
+    struct timespec relative_time= { 0, 0 };
+    pop_memc= memcached_pool_fetch(pool, &relative_time, &rc);
+
+    if (memcached_failed(rc))
+    {
+      test_null(pop_memc);
+    }
+  } while (rc == MEMCACHED_TIMEOUT and --counter);
+
+  if (memcached_failed(rc)) // Cleanup thread since we will exit once we test.
+  {
+    pthread_join(tid, NULL);
+    test_compare(MEMCACHED_SUCCESS, rc);
+  }
+
+  {
+    int pthread_ret= pthread_join(tid, NULL);
+    test_true(pthread_ret == 0 or pthread_ret == ESRCH);
+  }
+  test_compare(MEMCACHED_SUCCESS, rc);
+  test_true(pool_memc == pop_memc);
 
   test_true(memcached_pool_destroy(pool) == memc);
 
@@ -5910,6 +5959,7 @@ test_st tests[] ={
   {"analyzer", true, (test_callback_fn*)analyzer_test},
   {"memcached_pool_st", true, (test_callback_fn*)connection_pool_test },
   {"memcached_pool_st #2", true, (test_callback_fn*)connection_pool2_test },
+  {"memcached_pool_st #3", true, (test_callback_fn*)connection_pool3_test },
   {"memcached_pool_test", true, (test_callback_fn*)memcached_pool_test },
   {"test_get_last_disconnect", true, (test_callback_fn*)test_get_last_disconnect},
   {"verbosity", true, (test_callback_fn*)test_verbosity},