[PATCH 13/13] Check subscription database rather than explicit ADDSUB/DELSUB
Bron Gondwana
brong at fastmail.fm
Tue Jan 27 23:15:40 EST 2009
We were seeing occasional subscription mismatches at FastMail,
and the root cause was traced back to the re-ordering of log
files. All the ADDSUB commands would run first, then all the
DELSUB commands.
If a log file contained a DELSUB followed by an ADDSUB, they
would be re-ordered, and the subscription would exists on the
master but not the replica.
This patch changes the specialness of subscriptions, so that
instead of SUB and UNSUB events, it's just SUB events, and the
sync_client will check the subscription database to find what
the value should be, and tell the backend to make it so.
---
imap/cyr_synclog.c | 2 +-
imap/imapd.c | 2 +-
imap/mboxlist.c | 16 +++++++++++++
imap/mboxlist.h | 3 ++
imap/sync_client.c | 64 ++++++++++++++++++++-------------------------------
imap/sync_log.h | 5 +--
6 files changed, 48 insertions(+), 44 deletions(-)
diff --git a/imap/cyr_synclog.c b/imap/cyr_synclog.c
index 78489ac..b935c84 100644
--- a/imap/cyr_synclog.c
+++ b/imap/cyr_synclog.c
@@ -153,7 +153,7 @@ int main(int argc, char *argv[])
sync_log_seen(argv[optind], argv[optind+1]);
break;
case 'b': /* suBscription */
- sync_log_subscribe(argv[optind], argv[optind+1], argv[optind+2]);
+ sync_log_subscribe(argv[optind], argv[optind+1]);
break;
default:
/* just as is! */
diff --git a/imap/imapd.c b/imap/imapd.c
index 0bd144a..40acf66 100644
--- a/imap/imapd.c
+++ b/imap/imapd.c
@@ -6071,7 +6071,7 @@ void cmd_changesub(char *tag, char *namespace, char *name, int add)
else {
prot_printf(imapd_out, "%s OK %s\r\n", tag,
error_message(IMAP_OK_COMPLETED));
- sync_log_subscribe(imapd_userid, mailboxname, add);
+ sync_log_subscribe(imapd_userid, mailboxname);
}
}
diff --git a/imap/mboxlist.c b/imap/mboxlist.c
index 5cf0580..166eb4e 100644
--- a/imap/mboxlist.c
+++ b/imap/mboxlist.c
@@ -3311,6 +3311,22 @@ int mboxlist_findsub_alt(struct namespace *namespace,
return r;
}
+/* returns CYRUSDB_NOTFOUND if the folder doesn't exist, and 0 if it does! */
+int mboxlist_checksub(const char *name, const char *userid)
+{
+ int r;
+ struct db *subs;
+ char *val;
+ int vallen;
+
+ r = mboxlist_opensubs(userid, &subs);
+
+ if (!r) r = SUBDB->fetch(subs, name, strlen(name), &val, &vallen, NULL);
+
+ mboxlist_closesubs(subs);
+ return r;
+}
+
/*
* Change 'user's subscription status for mailbox 'name'.
* Subscribes if 'add' is nonzero, unsubscribes otherwise.
diff --git a/imap/mboxlist.h b/imap/mboxlist.h
index 59af143..b336a3d 100644
--- a/imap/mboxlist.h
+++ b/imap/mboxlist.h
@@ -179,6 +179,9 @@ int mboxlist_findsub_alt(struct namespace *namespace,
'stagedir' should be MAX_MAILBOX_PATH. */
int mboxlist_findstage(const char *name, char *stagedir, size_t sd_len);
+/* Check 'user's subscription status for mailbox 'name' */
+int mboxlist_checksub(const char *name, const char *userid);
+
/* Change 'user's subscription status for mailbox 'name'. */
int mboxlist_changesub(const char *name, const char *userid,
struct auth_state *auth_state, int add, int force);
diff --git a/imap/sync_client.c b/imap/sync_client.c
index d08f5c4..45fc8f5 100644
--- a/imap/sync_client.c
+++ b/imap/sync_client.c
@@ -88,6 +88,8 @@
#include "backend.h"
#include "xstrlcat.h"
#include "xstrlcpy.h"
+#include "signals.h"
+#include "cyrusdb.h"
/* signal to config.c */
const int config_need_data = 0; /* YYY */
@@ -697,40 +699,41 @@ static int folder_delete(char *name)
return(sync_parse_code("DELETE", fromserver, SYNC_PARSE_EAT_OKLINE, NULL));
}
-static int user_addsub(char *user, char *name)
+static int set_sub(char *user, char *name, int add)
{
+ char *cmd = add ? "ADDSUB" : "DELSUB";
+
if (verbose)
- printf("ADDSUB %s %s\n", user, name);
+ printf("%s %s %s\n", cmd, user, name);
if (verbose_logging)
- syslog(LOG_INFO, "ADDSUB %s %s", user, name);
+ syslog(LOG_INFO, "%s %s %s", cmd, user, name);
- prot_printf(toserver, "ADDSUB ");
+ sync_printastring(toserver, cmd);
+ prot_printf(toserver, " ");
sync_printastring(toserver, user);
prot_printf(toserver, " ");
sync_printastring(toserver, name);
prot_printf(toserver, "\r\n");
prot_flush(toserver);
- return(sync_parse_code("ADDSUB", fromserver, SYNC_PARSE_EAT_OKLINE, NULL));
+ return(sync_parse_code(cmd, fromserver, SYNC_PARSE_EAT_OKLINE, NULL));
}
-static int user_delsub(char *user, char *name)
+static int user_sub(char *user, char *name)
{
- if (verbose)
- printf("DELSUB %s %s\n", user, name);
-
- if (verbose_logging)
- syslog(LOG_INFO, "DELSUB %s %s", user, name);
+ int r;
- prot_printf(toserver, "DELSUB ");
- sync_printastring(toserver, user);
- prot_printf(toserver, " ");
- sync_printastring(toserver, name);
- prot_printf(toserver, "\r\n");
- prot_flush(toserver);
+ r = mboxlist_checksub(name, user);
- return(sync_parse_code("DELSUB", fromserver, SYNC_PARSE_EAT_OKLINE, NULL));
+ switch (r) {
+ case CYRUSDB_OK:
+ return set_sub(user, name, 1);
+ case CYRUSDB_NOTFOUND:
+ return set_sub(user, name, 0);
+ default:
+ return r;
+ }
}
static int folder_setacl(char *name, char *acl)
@@ -2354,7 +2357,7 @@ int do_user_sub(char *user, struct sync_folder_list *server_list)
do {
(sync_namespace.mboxname_tointernal)(&sync_namespace, s->name,
user, buf);
- if ((r = user_delsub(user, buf))) goto bail;
+ if ((r = set_sub(user, buf, 0))) goto bail;
s = s->next;
if (!s) n = -1; /* end of server list, we're done */
else if (!c) n = 1; /* remove all server subscriptions */
@@ -2368,7 +2371,7 @@ int do_user_sub(char *user, struct sync_folder_list *server_list)
}
else if (c && n < 0) {
/* add the current client subscription */
- if ((r = user_addsub(user, c->name))) goto bail;
+ if ((r = set_sub(user, c->name, 1))) goto bail;
}
}
@@ -2926,7 +2929,6 @@ static int do_sync(const char *filename)
struct sync_action_list *annot_list = sync_action_list_create();
struct sync_action_list *seen_list = sync_action_list_create();
struct sync_action_list *sub_list = sync_action_list_create();
- struct sync_action_list *unsub_list = sync_action_list_create();
struct sync_folder_list *folder_list = sync_folder_list_create();
static struct buf type, arg1, arg2;
char *arg1s, *arg2s;
@@ -3007,7 +3009,7 @@ static int do_sync(const char *filename)
else if (!strcmp(type.s, "SUB"))
sync_action_list_add(sub_list, arg2s, arg1s);
else if (!strcmp(type.s, "UNSUB"))
- sync_action_list_add(unsub_list, arg2s, arg1s);
+ sync_action_list_add(sub_list, arg2s, arg1s);
else
syslog(LOG_ERR, "Unknown action type: %s", type.s);
}
@@ -3033,7 +3035,6 @@ static int do_sync(const char *filename)
remove_meta(action->user, sieve_list);
remove_meta(action->user, seen_list);
remove_meta(action->user, sub_list);
- remove_meta(action->user, unsub_list);
}
for (action = meta_list->head ; action ; action = action->next) {
@@ -3042,7 +3043,6 @@ static int do_sync(const char *filename)
remove_meta(action->user, sieve_list);
remove_meta(action->user, seen_list);
remove_meta(action->user, sub_list);
- remove_meta(action->user, unsub_list);
}
for (action = mailbox_list->head ; action ; action = action->next) {
@@ -3159,7 +3159,7 @@ static int do_sync(const char *filename)
}
for (action = sub_list->head ; action ; action = action->next) {
- if (action->active && user_addsub(action->user, action->name)) {
+ if (action->active && user_sub(action->user, action->name)) {
sync_action_list_add(meta_list, NULL, action->user);
if (verbose) {
printf(" Promoting: SUB %s %s -> META %s\n",
@@ -3172,19 +3172,6 @@ static int do_sync(const char *filename)
}
}
- for (action = unsub_list->head ; action ; action = action->next) {
- if (action->active && user_delsub(action->user, action->name)) {
- sync_action_list_add(meta_list, NULL, action->user);
- if (verbose) {
- printf(" Promoting: UNSUB %s %s -> META %s\n",
- action->user, action->name, action->user);
- }
- if (verbose_logging) {
- syslog(LOG_INFO, " Promoting: UNSUB %s %s -> META %s",
- action->user, action->name, action->name);
- }
- }
- }
for (action = mailbox_list->head ; action ; action = action->next) {
if (!action->active)
continue;
@@ -3275,7 +3262,6 @@ static int do_sync(const char *filename)
sync_action_list_free(&annot_list);
sync_action_list_free(&seen_list);
sync_action_list_free(&sub_list);
- sync_action_list_free(&unsub_list);
sync_folder_list_free(&folder_list);
prot_free(input);
diff --git a/imap/sync_log.h b/imap/sync_log.h
index f1c267d..fb349e6 100644
--- a/imap/sync_log.h
+++ b/imap/sync_log.h
@@ -81,8 +81,7 @@ void sync_log(char *fmt, ...);
#define sync_log_seen(user, name) \
sync_log("SEEN %s %s\n", user, name)
-#define sync_log_subscribe(user, name, add) \
- if (add) sync_log("SUB %s %s\n", user, name); \
- else sync_log("UNSUB %s %s\n", user, name)
+#define sync_log_subscribe(user, name) \
+ sync_log("SUB %s %s\n", user, name)
#endif /* INCLUDED_SYNC_LOG_H */
--
1.5.6.3
More information about the Cyrus-devel
mailing list