More mupdate questions.

[Dave McMurtrie]

Wesley Craig wrote:
> On 30 Mar 2009, at 12:41, Dave McMurtrie wrote:
>> The fundamental problem is that kick_mupdate() returns void.  If a
>> mupdate slave is not currently listening on its local AF_UNIX socket
>> (like when it disconnects because it loses contact with the mupdate
>> master) kick_mupdate() will attempt to connect, fail, log an error and
>> return.  Most of the calling code I looked at assumes that if
>> kick_mupdate() returns, the local database is current.  In this case,
>> that may or may not be true.  At the very least, I think kick_mupdate()
>> needs to be updated to be able to return a status so any calling code
>> can make a more informed decision about what to do.
> 
> I don't have a big problem with more information being provided to the
> caller, but I'm not all that clear what you'll do with the information
> in most cases.  If the socket was open, you'd block waiting for the
> MUPDATE NOOP to happen.  With the socket closed, what are the superior
> behaviors?
> 
>> The area for improvement is that kick_mupdate() never times out its
>> read() from mupdate's local unix socket.  Wouldn't it be a good idea to
>> wrap this read() in an alarm() call and return failure?
> 
> See above.  I'd want to see an analysis of what each of the callers
> would do with the returned kick_mupdate() status.
>

That depends on what the purpose of kick_mupdate() is supposed to be.

The comments I found and the context around where it's called seem to
indicate that the caller is trying to ensure that the local mailbox copy
is up to date when it calls kick_mupdate().

Looking at the code in kick_mupdate() it looks like the only thing it
will ever successfully accomplish is causing a caller to wait until a
mailbox sync has completed if the mupdate slave is in the middle of a
sync operation.  Otherwise, it does nothing.  Worse, it fails to report
obvious error conditions to the caller.

I think the only really bad case where this could break something is in
lmtpd.c, inside mlookup().

This bit of code could cause a message to bounce:

        r = mboxlist_detail(name, &type, NULL, NULL, server, aclp, tid);
        if (r == IMAP_MAILBOX_NONEXISTENT && config_mupdate_server) {
            kick_mupdate();
            r = mboxlist_detail(name, &type, NULL, NULL, server, aclp, tid);

It does a local lookup, calls kick_mupdate(), then does the exact same
lookup again.  In the case of a network problem like I mentioned
previously, mupdate closes its local unix socket while it's attempting
to reconnect with the mupdate master.  Calls to kick_mupdate() will
simply return when the connect fails and mlookup() will return (possibly
incorrectly) IMAP_MAILBOX_NONEXISTENT to the caller.

Thoughts?

Thanks,

Dave
-- 
Dave McMurtrie, SPE
Email Systems Team Leader
Carnegie Mellon University,
Computing Services