[geary/wip/714802-chron: 2/3] Fix, simplify AccountSynchronizer
- From: Jim Nelson <jnelson src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/714802-chron: 2/3] Fix, simplify AccountSynchronizer
- Date: Sat, 24 Jan 2015 03:46:08 +0000 (UTC)
commit f057e65c99573ffe86498862fa2bb20418865e9e
Author: Jim Nelson <jim yorba org>
Date: Fri Jan 23 17:28:42 2015 -0800
Fix, simplify AccountSynchronizer
.../imap-engine-account-synchronizer.vala | 236 +++++++++-----------
.../imap-engine/imap-engine-minimal-folder.vala | 4 +-
.../imap-engine-abstract-list-email.vala | 3 +-
.../replay-ops/imap-engine-list-email-by-id.vala | 4 +-
.../imap-engine-server-search-email.vala | 23 +--
5 files changed, 117 insertions(+), 153 deletions(-)
---
diff --git a/src/engine/imap-engine/imap-engine-account-synchronizer.vala
b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
index de64620..97c8e85 100644
--- a/src/engine/imap-engine/imap-engine-account-synchronizer.vala
+++ b/src/engine/imap-engine/imap-engine-account-synchronizer.vala
@@ -9,10 +9,24 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
private const int SYNC_DELAY_SEC = 10;
private const int RETRY_SYNC_DELAY_SEC = 60;
+ private enum Reason {
+ MADE_AVAILABLE,
+ ALTERED
+ }
+
+ private class SyncMessage : BaseObject {
+ public MinimalFolder folder;
+ public Reason reason;
+
+ public SyncMessage(MinimalFolder folder, Reason reason) {
+ this.folder = folder;
+ this.reason = reason;
+ }
+ }
+
public GenericAccount account { get; private set; }
- private Nonblocking.Mailbox<MinimalFolder> bg_queue = new
Nonblocking.Mailbox<MinimalFolder>(bg_queue_comparator);
- private Gee.HashSet<MinimalFolder> made_available = new Gee.HashSet<MinimalFolder>();
+ private Nonblocking.Mailbox<SyncMessage> bg_queue = new
Nonblocking.Mailbox<SyncMessage>(bg_queue_comparator);
private MinimalFolder? current_folder = null;
private Cancellable? bg_cancellable = null;
private Nonblocking.Semaphore stopped = new Nonblocking.Semaphore();
@@ -78,10 +92,8 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
private void on_account_prefetch_changed() {
try {
- // treat as an availability check (i.e. as if the account had just opened) because
- // just because this value has changed doesn't mean the contents in the folders
- // have changed
- delayed_send_all(account.list_folders(), true, SYNC_DELAY_SEC);
+ // if prefetch value changes, reschedule all folders to account for new value
+ delayed_send_all(account.list_folders(), Reason.MADE_AVAILABLE, SYNC_DELAY_SEC);
} catch (Error err) {
debug("Unable to schedule re-sync for %s due to prefetch time changing: %s",
account.to_string(), err.message);
@@ -97,7 +109,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
foreach (Folder folder in available)
unavailable_paths.remove(folder.path);
- delayed_send_all(available, true, SYNC_DELAY_SEC);
+ delayed_send_all(available, Reason.MADE_AVAILABLE, SYNC_DELAY_SEC);
}
if (unavailable != null) {
@@ -109,35 +121,28 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
}
private void on_folders_contents_altered(Gee.Collection<Folder> altered) {
- delayed_send_all(altered, false, SYNC_DELAY_SEC);
+ delayed_send_all(altered, Reason.ALTERED, SYNC_DELAY_SEC);
}
private void on_email_sent() {
try {
Folder? sent_mail = account.get_special_folder(SpecialFolderType.SENT);
if (sent_mail != null)
- send_all(iterate<Folder>(sent_mail).to_array_list(), false);
+ send_all(iterate<Folder>(sent_mail).to_array_list(), Reason.ALTERED);
} catch (Error err) {
debug("Unable to retrieve Sent Mail from %s: %s", account.to_string(), err.message);
}
}
- private void delayed_send_all(Gee.Collection<Folder> folders, bool reason_available, int sec) {
+ private void delayed_send_all(Gee.Collection<Folder> folders, Reason reason, int sec) {
Timeout.add_seconds(sec, () => {
- // remove any unavailable folders
- Gee.ArrayList<Folder> trimmed_folders = new Gee.ArrayList<Folder>();
- foreach (Folder folder in folders) {
- if (!unavailable_paths.contains(folder.path))
- trimmed_folders.add(folder);
- }
-
- send_all(trimmed_folders, reason_available);
+ send_all(folders, reason);
return false;
});
}
- private void send_all(Gee.Collection<Folder> folders, bool reason_available) {
+ private void send_all(Gee.Collection<Folder> folders, Reason reason) {
foreach (Folder folder in folders) {
MinimalFolder? imap_folder = folder as MinimalFolder;
@@ -145,48 +150,65 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
if (imap_folder == null)
continue;
+ // don't bother with unselectable or local-only folders
+ if (!folder.properties.is_openable.is_possible() || folder.properties.is_local_only)
+ continue;
+
+ // drop unavailable folders
+ if (unavailable_paths.contains(folder.path))
+ continue;
+
// if considering folder not because it's available (i.e. because its contents changed),
// and the folder is open, don't process it; MinimalFolder will take care of changes as
// they occur, in order to remain synchronized
- if (imap_folder.get_open_state() != Folder.OpenState.CLOSED)
+ if (reason != Reason.MADE_AVAILABLE && folder.get_open_state() != Folder.OpenState.CLOSED)
continue;
// don't requeue the currently processing folder
- if (imap_folder != current_folder)
- bg_queue.send(imap_folder);
+ if (imap_folder == current_folder)
+ continue;
+
+ // if altered, remove made_available sync message; if made_available, comparator will
+ // drop if altered message already present; this prioritizes altered over made_available
+ if (reason == Reason.ALTERED) {
+ bg_queue.revoke_matching((msg) => {
+ return msg.folder == imap_folder && msg.reason == Reason.MADE_AVAILABLE;
+ });
+ }
- // If adding because now available, make sure it's flagged as such, since there's an
- // additional check for available folders ... if not, remove from the map so it's
- // not treated as such, in case both of these come in back-to-back
- if (reason_available && imap_folder != current_folder)
- made_available.add(imap_folder);
- else
- made_available.remove(imap_folder);
+ bg_queue.send(new SyncMessage(imap_folder, reason));
}
}
private void revoke_all(Gee.Collection<Folder> folders) {
foreach (Folder folder in folders) {
- MinimalFolder? generic_folder = folder as MinimalFolder;
- if (generic_folder != null) {
- bg_queue.revoke(generic_folder);
- made_available.remove(generic_folder);
- }
+ MinimalFolder? imap_folder = folder as MinimalFolder;
+ if (imap_folder == null)
+ continue;
+
+ bg_queue.revoke_matching((msg) => {
+ return msg.folder == imap_folder;
+ });
}
}
// This is used to ensure that certain special folders get prioritized over others, so folders
// important to the user (i.e. Inbox) go first while less-used folders (Spam) are fetched last
- private static int bg_queue_comparator(MinimalFolder a, MinimalFolder b) {
+ private static int bg_queue_comparator(SyncMessage a, SyncMessage b) {
if (a == b)
return 0;
- int cmp = score_folder(a) - score_folder(b);
+ // don't allow the same folder in the queue at the same time; it's up to the submitter to
+ // remove existing folders based on reason
+ if (a.folder == b.folder)
+ return 0;
+
+ int cmp = score_folder(a.folder) - score_folder(b.folder);
if (cmp != 0)
return cmp;
// sort by path to stabilize the sort
- return a.path.compare_to(b.path);
+ return a.folder.path.compare_to(b.folder.path);
}
// Lower the score, the higher the importance.
@@ -236,9 +258,9 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
private async void process_queue_async() {
for (;;) {
- MinimalFolder folder;
+ SyncMessage msg;
try {
- folder = yield bg_queue.recv_async(bg_cancellable);
+ msg = yield bg_queue.recv_async(bg_cancellable);
} catch (Error err) {
if (!(err is IOError.CANCELLED))
debug("Failed to receive next folder for background sync: %s", err.message);
@@ -247,7 +269,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
}
// mark as current folder to prevent requeues while processing
- current_folder = folder;
+ current_folder = msg.folder;
// generate the current epoch for synchronization (could cache this value, obviously, but
// doesn't seem like this biggest win in this class)
@@ -259,37 +281,36 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
epoch = max_epoch;
}
- bool ok = yield process_folder_async(folder, made_available.remove(folder), epoch);
+ bool not_cancelled = yield process_folder_async(msg.folder, msg.reason, epoch);
// clear current folder in every event
current_folder = null;
- if (!ok)
+ if (!not_cancelled)
break;
}
// clear queue of any remaining folders so references aren't held
bg_queue.clear();
- // same with made_available table
- made_available.clear();
-
// flag as stopped for any waiting tasks
stopped.blind_notify();
}
// Returns false if IOError.CANCELLED received
- private async bool process_folder_async(MinimalFolder folder, bool availability_check, DateTime epoch) {
- // get oldest local email and its time, as well as number of messages in local store
- DateTime? oldest_local = null;
+ private async bool process_folder_async(MinimalFolder folder, Reason reason, DateTime epoch) {
+ // get oldest local email; when folders are more-or-less synchronized, this reduces the
+ // IMAP SEARCH results by only asking for messages since a certain date that exist in the
+ // UID message set prior to this earliest one, reducing network traffic by not returning
+ // UIDs for messages already in the local store
+ DateTime? oldest_local_date = null;
Geary.EmailIdentifier? oldest_local_id = null;
int local_count = 0;
try {
Gee.List<Geary.Email>? list = yield folder.local_folder.list_email_by_id_async(null, 1,
- Email.Field.PROPERTIES, ImapDB.Folder.ListFlags.NONE |
ImapDB.Folder.ListFlags.OLDEST_TO_NEWEST,
- bg_cancellable);
+ Email.Field.PROPERTIES, ImapDB.Folder.ListFlags.OLDEST_TO_NEWEST, bg_cancellable);
if (list != null && list.size > 0) {
- oldest_local = list[0].properties.date_received;
+ oldest_local_date = list[0].properties.date_received;
oldest_local_id = list[0].id;
}
@@ -299,35 +320,26 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
debug("Unable to fetch oldest local email for %s: %s", folder.to_string(), err.message);
}
- if (availability_check) {
- // Compare the oldest mail in the local store and see if it is before the epoch; if so, no
- // need to synchronize simply because this Folder is available; wait for its contents to
- // change instead
- if (oldest_local != null) {
- if (oldest_local.compare(epoch) < 0) {
- // Oldest local email before epoch, don't sync from network
+ // altered folders will always been synchronized, but if only made available (i.e. the
+ // Account just opened up or the folder was just discovered), do some checking to avoid
+ // round-tripping to the server
+ if (reason == Reason.MADE_AVAILABLE) {
+ if (oldest_local_id != null) {
+ if (oldest_local_date.compare(epoch) < 0) {
+ // Oldest local is before epoch
return true;
} else if (folder.properties.email_total == local_count) {
- // Local earliest email is after epoch, but there's nothing before it
+ // Completely synchronized with remote
return true;
- } else {
- debug("Oldest local email in %s not old enough (%s vs. %s), email_total=%d vs.
local_count=%d, synchronizing...",
- folder.to_string(), oldest_local.to_string(), epoch.to_string(),
- folder.properties.email_total, local_count);
}
} else if (folder.properties.email_total == 0) {
- // no local messages, no remote messages -- this is as good as having everything up
- // to the epoch
+ // no local messages, no remote messages
return true;
- } else {
- debug("No oldest message found for %s, synchronizing...", folder.to_string());
}
- } else {
- debug("Folder %s changed, synchronizing...", folder.to_string());
}
try {
- yield folder.open_async(Folder.OpenFlags.FAST_OPEN, bg_cancellable);
+ yield folder.open_async(Folder.OpenFlags.NO_DELAY, bg_cancellable);
} catch (Error err) {
// don't need to close folder; if either calls throws an error, the folder is not open
if (err is IOError.CANCELLED)
@@ -336,14 +348,14 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
debug("Unable to open %s: %s", folder.to_string(), err.message);
// retry later
- delayed_send_all(iterate<Folder>(folder).to_array_list(), false, RETRY_SYNC_DELAY_SEC);
+ delayed_send_all(iterate<Folder>(folder).to_array_list(), reason, RETRY_SYNC_DELAY_SEC);
return true;
}
bool not_cancelled = true;
try {
- yield sync_folder_async(folder, epoch, oldest_local, oldest_local_id);
+ yield sync_folder_async(folder, epoch, oldest_local_id);
} catch (Error err) {
if (err is IOError.CANCELLED) {
not_cancelled = false;
@@ -351,7 +363,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
debug("Error background syncing folder %s: %s", folder.to_string(), err.message);
// retry later
- delayed_send_all(iterate<Folder>(folder).to_array_list(), false, RETRY_SYNC_DELAY_SEC);
+ delayed_send_all(iterate<Folder>(folder).to_array_list(), reason, RETRY_SYNC_DELAY_SEC);
}
// fallthrough and close
@@ -367,7 +379,7 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
return not_cancelled;
}
- private async void sync_folder_async(MinimalFolder folder, DateTime epoch, DateTime? oldest_local,
+ private async void sync_folder_async(MinimalFolder folder, DateTime epoch,
Geary.EmailIdentifier? oldest_local_id) throws Error {
debug("Background sync'ing %s", folder.to_string());
@@ -375,67 +387,23 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
// information
yield folder.wait_for_open_async(bg_cancellable);
- // only perform vector expansion if oldest isn't old enough
- if (oldest_local == null || oldest_local.compare(epoch) > 0) {
- // go back three months at a time to the epoch, performing a little vector expansion at a
- // time rather than all at once (which will stall the replay queue)
- DateTime current_epoch = (oldest_local != null) ? oldest_local : new DateTime.now_local();
- do {
- // look for complete synchronization of UIDs (i.e. complete vector normalization)
- // no need to keep searching once this happens
- int local_count = yield
folder.local_folder.get_email_count_async(ImapDB.Folder.ListFlags.NONE,
- bg_cancellable);
- if (local_count >= folder.properties.email_total) {
- debug("Total vector normalization for %s: %d/%d emails", folder.to_string(), local_count,
- folder.properties.email_total);
-
- break;
- }
-
- current_epoch = current_epoch.add_months(-1);
-
- // if past max_epoch, then just pull in everything and be done with it
- if (current_epoch.compare(max_epoch) < 0) {
- debug("Background sync reached max epoch of %s, fetching all mail from %s",
- max_epoch.to_string(), folder.to_string());
-
- yield folder.list_email_by_id_async(null, 1, Geary.Email.Field.NONE,
- Geary.Folder.ListFlags.OLDEST_TO_NEWEST, bg_cancellable);
- } else {
- // don't go past proscribed epoch
- if (current_epoch.compare(epoch) < 0)
- current_epoch = epoch;
-
- debug("Background sync'ing %s to %s", folder.to_string(), current_epoch.to_string());
- Geary.EmailIdentifier? earliest_span_id = yield
folder.find_earliest_email_async(current_epoch,
- oldest_local_id, bg_cancellable);
- if (earliest_span_id == null && current_epoch.compare(epoch) <= 0) {
- debug("Unable to locate epoch messages on remote folder %s%s, fetching one past
oldest...",
- folder.to_string(),
- (oldest_local_id != null) ? " earlier than oldest local" : "");
-
- // if there's nothing between the oldest local and the epoch, that means the
- // mail just prior to our local oldest is oldest than the epoch; rather than
- // continually thrashing looking for something that's just out of reach, add it
- // to the folder and be done with it ... note that this even works if oldest_local_id
- // is null, as that means the local folder is empty and so we should at least
- // pull the first one to get a marker of age
- yield folder.list_email_by_id_async(oldest_local_id, 1, Geary.Email.Field.NONE,
- Geary.Folder.ListFlags.NONE, bg_cancellable);
- } else if (earliest_span_id != null) {
- // use earliest email from that span for the next round
- oldest_local_id = earliest_span_id;
- }
- }
-
- yield Scheduler.sleep_ms_async(200);
- } while (current_epoch.compare(epoch) > 0);
- } else {
- debug("No expansion necessary for %s, oldest local (%s) is before epoch (%s)",
- folder.to_string(), oldest_local.to_string(), epoch.to_string());
- }
+ // find the first email (from 1 to n) that is since the epoch date but before the oldest local
+ // id; this will expand the vector in the database to accomodate it, pulling in the
+ // ImapDB.Folder.REQUIRED_FIELDS for all email from the end of the vector (n) to it
+ Geary.EmailIdentifier? earliest_id = yield folder.find_earliest_email_async(epoch,
+ oldest_local_id, bg_cancellable);
+
+ // found an email in the range that is older than epoch, list *one email earlier* than
+ // it to act as a sentinal; that email will be before the epoch and prevent future
+ // resynchronization when MADE_AVAILABLE triggers (which happens each startup) ...
+ // if not found, use the oldest_local_id to pull one before it to act as the sentinel;
+ // if both are null, use that, which will pull the most recent email for the folder and
+ // that becomes the sentinal
+ yield folder.list_email_by_id_async(earliest_id ?? oldest_local_id, 1, Email.Field.NONE,
+ Folder.ListFlags.NONE, bg_cancellable);
- // always give email prefetcher time to finish its work
+ // always give email prefetcher time to finish its work; this synchronizes the folder's
+ // contents with the (possibly expanded) vector
debug("Waiting for email prefetcher to complete %s...", folder.to_string());
try {
yield folder.email_prefetcher.active_sem.wait_async(bg_cancellable);
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 66d39b7..39d1e26 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -1388,8 +1388,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
notify_email_flags_changed(changed);
}
- // TODO: A proper public search mechanism; note that this always round-trips to the remote,
- // doesn't go through the replay queue, and doesn't deal with messages marked for deletion
+ // TODO: A proper public search mechanism; note that this always round-trips to the remote
+ // and ignores messages marked for deletion
internal async Geary.EmailIdentifier? find_earliest_email_async(DateTime datetime,
Geary.EmailIdentifier? before_id, Cancellable? cancellable) throws Error {
check_open("find_earliest_email_async");
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala
b/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala
index 758c29b..ac6cb58 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-abstract-list-email.vala
@@ -238,7 +238,8 @@ private abstract class Geary.ImapEngine.AbstractListEmail : Geary.ImapEngine.Sen
// if initial_id is null or no local earliest UID, then vector expansion is simple:
// merely count backwards from the top of the locally available vector
if (initial_uid == null || local_count == 0) {
- low_pos = new Imap.SequenceNumber(Numeric.int_floor((remote_count - local_count) - count,
1));
+ low_pos = new Imap.SequenceNumber(
+ Numeric.int_floor((remote_count - local_count) - count + 1, 1));
// don't set high_pos, leave null to use symbolic "highest" in MessageSet
high_pos = null;
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala
b/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala
index af07b35..f8a1c2c 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-list-email-by-id.vala
@@ -101,7 +101,7 @@ private class Geary.ImapEngine.ListEmailByID : Geary.ImapEngine.AbstractListEmai
if (is_fully_expanded == Trillian.FALSE) {
if (flags.is_oldest_to_newest()) {
if (initial_id != null) {
- // expand vector if not initial_id not discovered
+ // expand vector if initial uid is unknown
expansion_required = (initial_uid == null);
} else {
// initial_id == null, expansion required if not fully already
@@ -113,7 +113,7 @@ private class Geary.ImapEngine.ListEmailByID : Geary.ImapEngine.AbstractListEmai
// if infinite count, expansion required if not already
expansion_required = true;
} else if (initial_id != null) {
- // finite count, expansion required if initial not found *or* not enough
+ // finite count, expansion required if initial UID not found *or* not enough
// items were pulled in
expansion_required = (initial_uid == null) || (fulfilled_count + get_unfulfilled_count()
< count);
} else {
diff --git a/src/engine/imap-engine/replay-ops/imap-engine-server-search-email.vala
b/src/engine/imap-engine/replay-ops/imap-engine-server-search-email.vala
index 5b1443d..2dd2cfa 100644
--- a/src/engine/imap-engine/replay-ops/imap-engine-server-search-email.vala
+++ b/src/engine/imap-engine/replay-ops/imap-engine-server-search-email.vala
@@ -39,22 +39,17 @@ private class Geary.ImapEngine.ServerSearchEmail : Geary.ImapEngine.AbstractList
yield expand_vector_async(uids.first(), 1);
// Convert UIDs into EmailIdentifiers for lookup
- Gee.HashSet<ImapDB.EmailIdentifier> local_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
- foreach (Imap.UID uid in uids) {
- // if null, presumably was picked up in the vector expansion (but hasn't been assigned
- // to the database yet)
- //
- // TODO: We need a sparse version of this to scoop them up all at once
- ImapDB.EmailIdentifier? id = yield owner.local_folder.get_id_async(uid,
- ImapDB.Folder.ListFlags.NONE, cancellable);
- if (id != null)
- local_ids.add(id);
- }
+ Gee.Set<ImapDB.EmailIdentifier>? local_ids = yield owner.local_folder.get_ids_async(uids,
+ ImapDB.Folder.ListFlags.NONE, cancellable);
- Gee.List<Geary.Email>? local_list = yield owner.local_folder.list_email_by_sparse_id_async(
- local_ids, required_fields, ImapDB.Folder.ListFlags.PARTIAL_OK, cancellable);
+ // Fetch what is in local store currently for those UIDs
+ Gee.List<Geary.Email>? local_list = null;
+ if (local_ids != null) {
+ local_list = yield owner.local_folder.list_email_by_sparse_id_async(local_ids, required_fields,
+ ImapDB.Folder.ListFlags.PARTIAL_OK, cancellable);
+ }
- // Build list of local email
+ // Build map of local email
Gee.Map<ImapDB.EmailIdentifier, Geary.Email> map = new Gee.HashMap<ImapDB.EmailIdentifier,
Geary.Email>();
if (local_list != null) {
foreach (Geary.Email email in local_list)
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
