CORRECT PATCH Re: sync_client bails when encountering a deleted
message
John Capo
jc at irbs.com
Thu May 17 20:22:41 EDT 2007
Quoting Ken Murchison (murch at andrew.cmu.edu):
> John Capo wrote:
> >Quoting Ken Murchison (murch at andrew.cmu.edu):
> >>John Capo wrote:
> >>>Quoting John Capo (jc at irbs.com):
> >>>>Quoting Ken Murchison (murch at andrew.cmu.edu):
> >>>>>I don't think I was clear. With my proposal, we're well past
> >>>>>"UPDATE". I'm talking about cutting the command short after
> >>>>>"(<flags>)". No header_size or anything after.
> >>>>I guess I don't understand what you mean.
> >>>>
> >>>>It sure looks to me like cmd_upload() has to detect that the header
> >>>>size it is waiting for will not arrive and recover gracefully rather
> >>>>than sending a BAD that the client sees as the response for its
> >>>>next command. Patch attached.
> >>>The server needs to push the character back except on EOF, grrrr.
> >>Here's what I was thinking. We might be able to bump the cache flush
> >>down to parse_err, but I'm not sure if this is correct/suboptimal for
> >>SIMPLE.
> >
> >Ths might work if you add an eatline() in the client before returning.
> >The BAD response from the server must be discarded somehow or the
> >BAD response will be seen by the client as the response to the next
> >client command. It seems to me that no response is needed since
> >the client is not expecting one.
>
> Here is an untested patch which SHOULD allow the client to abort the
> UPLOAD, with the server successfully accepting the messages transmitted
> up to the abort.
>
> Thoughts?
This patch works fine on my test box when a message disappears. I
did not test folders disappearing but that should work too. Next
week I will update my production servers to use this patch instead
of my fix.
I have one customer that gets 5K+ messages daily with 350+ archive
folders. This customer is a really good stress test for the
sync_client.
> Index: sync_client.c
> ===================================================================
> RCS file: /afs/andrew/system/cvs/src/cyrus/imap/sync_client.c,v
> retrieving revision 1.10
> diff -u -r1.10 sync_client.c
> --- sync_client.c 7 May 2007 16:23:21 -0000 1.10
> +++ sync_client.c 17 May 2007 15:55:35 -0000
> @@ -1177,6 +1177,7 @@
> if (r) {
> syslog(LOG_ERR, "IOERROR: opening message file %lu of %s: %m",
> record->uid, mailbox->name);
> + sync_msgid_remove(msgid_onserver, &record->uuid);
> return(IMAP_IOERROR);
> }
>
> @@ -1246,7 +1247,7 @@
>
> /* Message with this UUID exists on client but not server */
> if ((r=upload_message_work(mailbox, msgno, &record)))
> - return(r);
> + break;
>
> if (msg && (msg->uid == record.uid)) /* Overwritten on server */
> msg = msg->next;
> @@ -1321,7 +1322,7 @@
> mailbox->last_uid, mailbox->last_appenddate);
>
> if ((r=upload_message_work(mailbox, msgno, &record)))
> - return(r);
> + break;
> }
>
> if (count == 0)
> Index: sync_server.c
> ===================================================================
> RCS file: /afs/andrew/system/cvs/src/cyrus/imap/sync_server.c,v
> retrieving revision 1.4
> diff -u -r1.4 sync_server.c
> --- sync_server.c 4 Apr 2007 15:22:42 -0000 1.4
> +++ sync_server.c 17 May 2007 15:55:35 -0000
> @@ -1889,8 +1889,9 @@
> break;
> case MSG_PARSED:
> if (c != ' ') {
> - err = "Invalid flags";
> - goto parse_err;
> + /* Missing message - UPLOAD short-circuited by client */
> + sync_upload_list_remove(upload_list, item);
> + goto done;
> }
>
> message = sync_message_add(message_list, &item->uuid);
> @@ -1943,6 +1944,7 @@
> /* if we see a SP, we're trying to upload more than one message */
> } while (c == ' ');
>
> + done:
> if (c == EOF) {
> err = "Unexpected end of sync_in at end of item";
> goto parse_err;
> Index: sync_support.c
> ===================================================================
> RCS file: /afs/andrew/system/cvs/src/cyrus/imap/sync_support.c,v
> retrieving revision 1.7
> diff -u -r1.7 sync_support.c
> --- sync_support.c 23 Apr 2007 14:15:32 -0000 1.7
> +++ sync_support.c 17 May 2007 15:55:35 -0000
> @@ -511,6 +511,22 @@
> return(result);
> }
>
> +void sync_msgid_remove(struct sync_msgid_list *l,
> + struct message_uuid *uuid)
> +{
> + int offset = message_uuid_hash(uuid, l->hash_size);
> + struct sync_msgid *msgid;
> +
> + if (message_uuid_isnull(uuid)) return;
> +
> + for (msgid = l->hash[offset] ; msgid ; msgid = msgid->hash_next) {
> + if (message_uuid_compare(&msgid->uuid, uuid)) {
> + message_uuid_set_null(&msgid->uuid);
> + return;
> + }
> + }
> +}
> +
> void sync_msgid_list_free(struct sync_msgid_list **lp)
> {
> struct sync_msgid_list *l = *lp;
> @@ -1363,6 +1379,27 @@
> l->count++;
>
> return(result);
> +}
> +
> +void sync_upload_list_remove(struct sync_upload_list *l,
> + struct sync_upload_item *i)
> +{
> + struct sync_upload_item *prev, *current, *next;
> +
> + prev = NULL;
> + current = l->head;
> + while (current) {
> + next = current->next;
> + if (current == i) {
> + l->count--;
> + free(current);
> + if (prev) prev->next = next;
> + else l->head = next;
> + return;
> + }
> + prev = current;
> + current = next;
> + }
> }
>
> void sync_upload_list_free(struct sync_upload_list **lp)
> Index: sync_support.h
> ===================================================================
> RCS file: /afs/andrew/system/cvs/src/cyrus/imap/sync_support.h,v
> retrieving revision 1.2
> diff -u -r1.2 sync_support.h
> --- sync_support.h 30 Nov 2006 17:11:20 -0000 1.2
> +++ sync_support.h 17 May 2007 15:55:35 -0000
> @@ -144,6 +144,9 @@
> struct sync_msgid *sync_msgid_add(struct sync_msgid_list *list,
> struct message_uuid *uuid);
>
> +void sync_msgid_remove(struct sync_msgid_list *l,
> + struct message_uuid *uuid);
> +
> struct sync_msgid *sync_msgid_lookup(struct sync_msgid_list *list,
> struct message_uuid *uuid);
>
> @@ -342,6 +345,9 @@
> char **flagname);
>
> struct sync_upload_item *sync_upload_list_add(struct sync_upload_list *l);
> +
> +void sync_upload_list_remove(struct sync_upload_list *l,
> + struct sync_upload_item *i);
>
> void sync_upload_list_free(struct sync_upload_list **lp);
>
>
> --
> Kenneth Murchison
> Systems Programmer
> Project Cyrus Developer/Maintainer
> Carnegie Mellon University
More information about the Cyrus-devel
mailing list