Merge lp:~mattknox/libmemcached/twitter-compat into lp:~tangent-org/libmemcached/trunk

[matt knox]

[matt knox (mattknox)]

The code in this branch caches memcached server errors in the memcached_server_st struct to give better visibility into failures. I've tested it both with its own suite and that of the ruby interface to it, and also run it in a development environment. So far it looks fine on all counts.

[lp:~mattknox/libmemcached/twitter-compat]

543. By matt knox on 2009-06-30

oops! Now I'm using the malloc api, rather than direct malloc

544. By matt knox on 2009-06-30

removed free

[Brian Aker (brianaker)]

> The code in this branch caches memcached server errors in the
> memcached_server_st struct to give better visibility into failures. I've
> tested it both with its own suite and that of the ruby interface to it, and
> also run it in a development environment. So far it looks fine on all counts.

There are a few problems with this.

1) You don't test for memory failures.
2) Lets say you have a failure, how do you know which server it was that had the error?
3) How do you clean up the error (aka reset the error)?
4) There is no function to encapsulate the error message. Which means any future changes to the structure are going to break if any renaming occurs.

I'll clean this up a bit... but something needs to be figured out so that you know via some vector about which servers got errors in the last run.

I can certainly see the usefulness in this for debugging on production sites.

[lp:~mattknox/libmemcached/twitter-compat]

545. By matt knox on 2009-07-13

corrected a spelling error in libmemcached_examples

[matt knox (mattknox)]

hey-I just made what I think are some improvements to the twitter-compat
branch. Among other things, I pulled the setting of the cached_server_error
out into its own function. I'll wrap the access also.

On Wed, Jul 15, 2009 at 2:38 AM, Brian Aker <email address hidden> wrote:

> > The code in this branch caches memcached server errors in the
> > memcached_server_st struct to give better visibility into failures. I've
> > tested it both with its own suite and that of the ruby interface to it,
> and
> > also run it in a development environment. So far it looks fine on all
> counts.
>
> There are a few problems with this.
>
> 1) You don't test for memory failures.
> 2) Lets say you have a failure, how do you know which server it was that
> had the error?
> 3) How do you clean up the error (aka reset the error)?
> 4) There is no function to encapsulate the error message. Which means any
> future changes to the structure are going to break if any renaming occurs.
>
> I'll clean this up a bit... but something needs to be figured out so that
> you know via some vector about which servers got errors in the last run.
>
> I can certainly see the usefulness in this for debugging on production
> sites.
>
>
> --
>
> https://code.launchpad.net/~mattknox/libmemcached/twitter-compat/+merge/8030<https://code.launchpad.net/%7Emattknox/libmemcached/twitter-compat/+merge/8030>
> You are the owner of lp:~mattknox/libmemcached/twitter-compat.
>

--
(def (eval e l d c)
 (if (atom? e)
     ((ahandler (type e)) e l d c)
     (eval (car e) l d
             (fun (x)
                (evapp x (cdr e) l d c)))))

[matt knox (mattknox)]

I've made several changes, including wrapping the cached_server_error in a
function and moving all that out into its own set of files, with docs and
such. How's that look now?

matt

On Wed, Jul 15, 2009 at 2:38 AM, Brian Aker <email address hidden> wrote:

> > The code in this branch caches memcached server errors in the
> > memcached_server_st struct to give better visibility into failures. I've
> > tested it both with its own suite and that of the ruby interface to it,
> and
> > also run it in a development environment. So far it looks fine on all
> counts.
>
> There are a few problems with this.
>
> 1) You don't test for memory failures.
> 2) Lets say you have a failure, how do you know which server it was that
> had the error?
> 3) How do you clean up the error (aka reset the error)?
> 4) There is no function to encapsulate the error message. Which means any
> future changes to the structure are going to break if any renaming occurs.
>
> I'll clean this up a bit... but something needs to be figured out so that
> you know via some vector about which servers got errors in the last run.
>
> I can certainly see the usefulness in this for debugging on production
> sites.
>
>
> --
>
> https://code.launchpad.net/~mattknox/libmemcached/twitter-compat/+merge/8030<https://code.launchpad.net/%7Emattknox/libmemcached/twitter-compat/+merge/8030>
> You are the owner of lp:~mattknox/libmemcached/twitter-compat.
>

--
(def (eval e l d c)
 (if (atom? e)
     ((ahandler (type e)) e l d c)
     (eval (car e) l d
             (fun (x)
                (evapp x (cdr e) l d c)))))

=== modified file 'libmemcached/memcached_response.c'
--- libmemcached/memcached_response.c	2009-06-15 00:51:32 +0000
+++ libmemcached/memcached_response.c	2009-06-29 20:51:23 +0000
@@ -238,7 +238,16 @@
         return MEMCACHED_STAT;
       }
       else if (buffer[1] == 'E')
-        return MEMCACHED_SERVER_ERROR;
+	{
+	  /* SERVER_ERROR */ 
+	  char *startptr= buffer + 13, *endptr= startptr;
+	  while (*endptr != '\r' && *endptr != '\n') endptr++;
+	  if (ptr->cached_server_error) free(ptr->cached_server_error);
+	  ptr->cached_server_error= malloc(endptr - startptr + 1);
+	  memcpy(ptr->cached_server_error, startptr, endptr - startptr);
+	  ptr->cached_server_error[endptr - startptr]= 0;
+	  return MEMCACHED_SERVER_ERROR;
+	}
       else if (buffer[1] == 'T')
         return MEMCACHED_STORED;
       else
 
=== modified file 'libmemcached/memcached_server.c'
--- libmemcached/memcached_server.c	2009-06-10 10:29:06 +0000
+++ libmemcached/memcached_server.c	2009-06-29 20:51:23 +0000
@@ -53,6 +53,12 @@
 {
   memcached_quit_server(ptr, 0);
 
+  if (ptr->cached_server_error)
+  {
+    free(ptr->cached_server_error);
+    ptr->cached_server_error= NULL;
+  }
+
   if (ptr->address_info)
   {
     freeaddrinfo(ptr->address_info);
@@ -70,14 +76,24 @@
 */
 memcached_server_st *memcached_server_clone(memcached_server_st *clone, memcached_server_st *ptr)
 {
+  memcached_server_st *rv = NULL;
+
   /* We just do a normal create if ptr is missing */
   if (ptr == NULL)
     return NULL;
 
-  /* TODO We should check return type */
-  return memcached_server_create_with(ptr->root, clone, 
-                                      ptr->hostname, ptr->port, ptr->weight,
-                                      ptr->type);
+  rv = memcached_server_create_with(ptr->root, clone, 
+                                    ptr->hostname, ptr->port, ptr->weight,
+                                    ptr->type);
+  if (rv != NULL)
+  {
+    rv->cached_errno= ptr->cached_errno;
+    if (ptr->cached_server_error)
+      rv->cached_server_error= strdup(ptr->cached_server_error);
+  }
+
+  return rv;
+
 }
 
 memcached_return memcached_server_cursor(memcached_st *ptr, 
 
=== modified file 'libmemcached/memcached_server.h'
--- libmemcached/memcached_server.h	2009-05-20 15:58:02 +0000
+++ libmemcached/memcached_server.h	2009-06-29 20:51:23 +0000
@@ -21,6 +21,7 @@
   unsigned int port;
   int cached_errno;
   int fd;
+  char *cached_server_error;
   uint32_t io_bytes_sent; /* # bytes sent since last read */
   uint32_t server_failure_counter;
   uint32_t weight;