Code review comment for lp:~mm-yuhu/gearmand/server-funcs

Hey!

Use of strcasecmp is not an issue, it's already there in the other
text parsing commands and we've not hit problems on any platform we
care about yet.

The bigger issue here is that jobs that are in the queue for a
function being removed will have dangling pointers. For example,
even with no workers, there may still be clients with jobs that
reference the function objects. These need to be checked and either
also removed, which means responding to client connections that some
error has occurred with the job, or simply not deleting the function
if jobs exist.

-Eric

On Thu, Mar 11, 2010 at 04:57:16PM -0000, Brian Aker wrote:
> Hi!
>
> A couple of things:
>
> 1) strcasecmp() is not found on all platforms.
>
> 2) We need a test case.
>
> Thanks!
> -Brian
>
> On Mar 11, 2010, at 5:22 AM, HackMan wrote:
>
> > HackMan has proposed merging lp:~mm-yuhu/gearmand/mm-dev into
> > lp:gearmand.
> >
> > Requested reviews:
> > Gearman-developers (gearman-developers)
> >
> >
> > Every now and then during development one hit a situation where a
> > client connects to a job server and requests a function to be
> > executed. If the client uses dispatch_background then the server
> > will wait depending on the configuration for a worker to do the job.
> >
> > However in my case the client has issued wrong arguments and I don't
> > want to connect the worker before removing the wrong requests and I
> > don't want to restart the job server just for that.
> >
> > This patch introduces a new command rmfunc which has only one
> > argument, the function name that we want to remove. If there are
> > connected workers, for this function we do not allow removing of the
> > function.
> > --
> > https://code.launchpad.net/~mm-yuhu/gearmand/mm-dev/+merge/21142
> > Your team Gearman-developers is subscribed to branch lp:gearmand.
> > === modified file 'libgearman-server/server.c'
> > --- libgearman-server/server.c 2010-01-28 21:45:14 +0000
> > +++ libgearman-server/server.c 2010-03-11 13:22:17 +0000
> > @@ -849,6 +849,34 @@
> > if (size < total)
> > snprintf(data + size, total - size, ".\n");
> > }
> > + else if (!strcasecmp("rmfunc", (char *)(packet->arg[0])))
> > + {
> > + if (packet->argc == 1)
> > + {
> > + snprintf(data, GEARMAN_TEXT_RESPONSE_SIZE, "ERR
> > incomplete_args "
> > + "An+incomplete+set+of+arguments+was+sent+to+this+command\n");
> > + }
> > + else
> > + {
> > + for (function= server_con->thread->server->function_list;
> > + function != NULL; function= function->next)
> > + {
> > + if (!strcasecmp(function->function_name, (char *)(packet-
> > >arg[1])))
> > + {
> > + if ( function->worker_count == 0 )
> > + {
> > + gearman_server_function_free(function);
> > + snprintf(data, GEARMAN_TEXT_RESPONSE_SIZE, "OK\n");
> > + }
> > + else
> > + {
> > + snprintf(data, GEARMAN_TEXT_RESPONSE_SIZE, "ERR there
> > are still connected workers\n");
> > + }
> > + }
> > + }
> > + }
> > + }
> > +
> > else if (!strcasecmp("maxqueue", (char *)(packet->arg[0])))
> > {
> > if (packet->argc == 1)
> >
>
> --
> https://code.launchpad.net/~mm-yuhu/gearmand/server-funcs/+merge/21142
> Your team Gearman-developers is subscribed to branch lp:gearmand.