Code review comment for lp:~mattknox/libmemcached/twitter-compat

Revision history for this message
matt knox (mattknox) wrote :

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)))))

« Back to merge proposal