# HG changeset patch # User Timo Sirainen # Date 1511596891 -7200 # Node ID c9549bea910652ebb013bfc280aee500daa12d08 # Parent d689d7d99b4fbed8f74700a78ae9cb53088b064c director: Don't send USERs in handshake that were already sent between handshake If the user was refreshed since the handshake was started, it means that the same user was already sent to the other side (added to the stream immediately after it was received/handled). There's no need to send it again. This fixes a potentally infinite handshake when users are rapidly changing and the handshake iterator never sees the end of the list. (Each refreshed user is moved to the end of the list, so handshaking can keep sending the same user over and over again.) diff -r d689d7d99b4f -r c9549bea9106 src/director/director-connection.c --- a/src/director/director-connection.c Sat Nov 25 10:05:27 2017 +0200 +++ b/src/director/director-connection.c Sat Nov 25 10:01:31 2017 +0200 @@ -2255,7 +2255,16 @@ director_connection_send_hosts(conn); i_assert(conn->user_iter == NULL); - conn->user_iter = director_iterate_users_init(conn->dir); + /* Iterate only through users that aren't refreshed since the + iteration started. The refreshed users will already be sent as + regular USER updates, so they don't need to be sent again. + + We especially don't want to send these users again, because + otherwise in a rapidly changing director we might never end up + sending all the users when they constantly keep being added to the + end of the list. (The iteration lists users in order from older to + newer.) */ + conn->user_iter = director_iterate_users_init(conn->dir, TRUE); if (director_connection_send_users(conn) == 0) o_stream_set_flush_pending(conn->output, TRUE); diff -r d689d7d99b4f -r c9549bea9106 src/director/director.c --- a/src/director/director.c Sat Nov 25 10:05:27 2017 +0200 +++ b/src/director/director.c Sat Nov 25 10:01:31 2017 +0200 @@ -1520,12 +1520,15 @@ struct director *dir; unsigned int tag_idx; struct user_directory_iter *user_iter; + bool iter_until_current_tail; }; -struct director_user_iter *director_iterate_users_init(struct director *dir) +struct director_user_iter * +director_iterate_users_init(struct director *dir, bool iter_until_current_tail) { struct director_user_iter *iter = i_new(struct director_user_iter, 1); iter->dir = dir; + iter->iter_until_current_tail = iter_until_current_tail; return iter; } @@ -1541,7 +1544,8 @@ if (iter->tag_idx >= array_count(tags)) return NULL; struct mail_tag *const *tagp = array_idx(tags, iter->tag_idx); - iter->user_iter = user_directory_iter_init((*tagp)->users); + iter->user_iter = user_directory_iter_init((*tagp)->users, + iter->iter_until_current_tail); } user = user_directory_iter_next(iter->user_iter); if (user == NULL) { diff -r d689d7d99b4f -r c9549bea9106 src/director/director.h --- a/src/director/director.h Sat Nov 25 10:05:27 2017 +0200 +++ b/src/director/director.h Sat Nov 25 10:01:31 2017 +0200 @@ -266,7 +266,8 @@ void dir_debug(const char *fmt, ...) ATTR_FORMAT(1, 2); -struct director_user_iter *director_iterate_users_init(struct director *dir); +struct director_user_iter * +director_iterate_users_init(struct director *dir, bool iter_until_current_tail); struct user *director_iterate_users_next(struct director_user_iter *iter); void director_iterate_users_deinit(struct director_user_iter **_iter); diff -r d689d7d99b4f -r c9549bea9106 src/director/doveadm-connection.c --- a/src/director/doveadm-connection.c Sat Nov 25 10:05:27 2017 +0200 +++ b/src/director/doveadm-connection.c Sat Nov 25 10:01:31 2017 +0200 @@ -522,7 +522,7 @@ director_connection_cork(dir->right); if (cmd->iter == NULL) { - cmd->iter = director_iterate_users_init(dir); + cmd->iter = director_iterate_users_init(dir, FALSE); cmd->users_killed = FALSE; } @@ -715,7 +715,7 @@ ip.family = 0; } - iter = director_iterate_users_init(conn->dir); + iter = director_iterate_users_init(conn->dir, FALSE); while ((user = director_iterate_users_next(iter)) != NULL) { if (ip.family == 0 || net_ip_compare(&ip, &user->host->ip)) T_BEGIN { diff -r d689d7d99b4f -r c9549bea9106 src/director/test-user-directory.c --- a/src/director/test-user-directory.c Sat Nov 25 10:05:27 2017 +0200 +++ b/src/director/test-user-directory.c Sat Nov 25 10:01:31 2017 +0200 @@ -19,7 +19,7 @@ struct user *user, *prev = NULL; unsigned int prev_stamp = 0, iter_count = 0; - iter = user_directory_iter_init(dir); + iter = user_directory_iter_init(dir, FALSE); while ((user = user_directory_iter_next(iter)) != NULL) { test_assert(prev_stamp <= user->timestamp); test_assert(user->prev == prev); diff -r d689d7d99b4f -r c9549bea9106 src/director/user-directory.c --- a/src/director/user-directory.c Sat Nov 25 10:05:27 2017 +0200 +++ b/src/director/user-directory.c Sat Nov 25 10:01:31 2017 +0200 @@ -17,7 +17,7 @@ struct user_directory_iter { struct user_directory *dir; - struct user *pos; + struct user *pos, *stop_after_tail; }; struct user_directory { @@ -46,6 +46,10 @@ array_foreach(&dir->iters, iterp) { if ((*iterp)->pos == user) (*iterp)->pos = user->next; + if ((*iterp)->stop_after_tail == user) { + (*iterp)->stop_after_tail = + user->prev != NULL ? user->prev : user->next; + } } } @@ -291,13 +295,15 @@ } struct user_directory_iter * -user_directory_iter_init(struct user_directory *dir) +user_directory_iter_init(struct user_directory *dir, + bool iter_until_current_tail) { struct user_directory_iter *iter; iter = i_new(struct user_directory_iter, 1); iter->dir = dir; iter->pos = dir->head; + iter->stop_after_tail = iter_until_current_tail ? dir->tail : NULL; array_append(&dir->iters, &iter, 1); user_directory_drop_expired(dir); return iter; @@ -312,6 +318,10 @@ return NULL; iter->pos = user->next; + if (user == iter->stop_after_tail) { + /* this is the last user we want to iterate */ + iter->pos = NULL; + } return user; } diff -r d689d7d99b4f -r c9549bea9106 src/director/user-directory.h --- a/src/director/user-directory.h Sat Nov 25 10:05:27 2017 +0200 +++ b/src/director/user-directory.h Sat Nov 25 10:01:31 2017 +0200 @@ -62,10 +62,15 @@ struct user *user); /* Iterate through users in the directory. It's safe to modify user directory - while iterators are running. The moved/removed users will just be skipped - over. */ + while iterators are running. The removed users will just be skipped over. + Users that are refreshed (= moved to end of list) may be processed twice. + + Using iter_until_current_tail=TRUE causes the iterator to not iterate + through any users that were added/refreshed since the iteration began. + Note that this may skip some users entirely. */ struct user_directory_iter * -user_directory_iter_init(struct user_directory *dir); +user_directory_iter_init(struct user_directory *dir, + bool iter_until_current_tail); struct user *user_directory_iter_next(struct user_directory_iter *iter); void user_directory_iter_deinit(struct user_directory_iter **iter);