From f7104bbcefc160c073f6ee6b8d4f37696731a4c0 Mon Sep 17 00:00:00 2001 From: Tom Hacohen Date: Fri, 12 May 2017 14:32:04 +0100 Subject: [PATCH] Syncmanager: fetch journal entries in chunks. Before this commit we used to fetch the whole journal entry list in one go, which caused issues in two cases: 1. On slow internet connections the download may fail. 2. With big journals: Android interrupts sync managers if they don't perform any significant network traffic for over a minute[1], and because we would first download and only then process, we would sometimes hit this threshold. Current chunk size is set to 50. 1: https://developer.android.com/reference/android/content/AbstractThreadedSyncAdapter.html --- .../journalmanager/JournalEntryManager.java | 6 +++- .../syncadapter/CalendarSyncManager.java | 2 -- .../syncadapter/ContactsSyncManager.java | 2 -- .../syncadapter/syncadapter/SyncManager.java | 30 +++++++++++-------- .../journalmanager/ServiceTest.java | 6 ++-- 5 files changed, 25 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/com/etesync/syncadapter/journalmanager/JournalEntryManager.java b/app/src/main/java/com/etesync/syncadapter/journalmanager/JournalEntryManager.java index 24928f31..73cea667 100644 --- a/app/src/main/java/com/etesync/syncadapter/journalmanager/JournalEntryManager.java +++ b/app/src/main/java/com/etesync/syncadapter/journalmanager/JournalEntryManager.java @@ -36,7 +36,7 @@ public class JournalEntryManager extends BaseManager { this.client = httpClient; } - public List list(Crypto.CryptoManager crypto, String last) throws Exceptions.HttpException, Exceptions.IntegrityException { + public List list(Crypto.CryptoManager crypto, String last, int limit) throws Exceptions.HttpException, Exceptions.IntegrityException { Entry previousEntry = null; HttpUrl.Builder urlBuilder = this.remote.newBuilder(); if (last != null) { @@ -44,6 +44,10 @@ public class JournalEntryManager extends BaseManager { previousEntry = Entry.getFakeWithUid(last); } + if (limit > 0) { + urlBuilder.addQueryParameter("limit", String.valueOf(limit)); + } + HttpUrl remote = urlBuilder.build(); Request request = new Request.Builder() diff --git a/app/src/main/java/com/etesync/syncadapter/syncadapter/CalendarSyncManager.java b/app/src/main/java/com/etesync/syncadapter/syncadapter/CalendarSyncManager.java index 00b4a807..9b3c4183 100644 --- a/app/src/main/java/com/etesync/syncadapter/syncadapter/CalendarSyncManager.java +++ b/app/src/main/java/com/etesync/syncadapter/syncadapter/CalendarSyncManager.java @@ -41,8 +41,6 @@ import okhttp3.HttpUrl; *

Synchronization manager for CardDAV collections; handles contacts and groups.

*/ public class CalendarSyncManager extends SyncManager { - protected static final int MAX_MULTIGET = 10; - final private HttpUrl remote; public CalendarSyncManager(Context context, Account account, AccountSettings settings, Bundle extras, String authority, SyncResult result, LocalCalendar calendar, HttpUrl remote) throws Exceptions.IntegrityException, Exceptions.GenericCryptoException { diff --git a/app/src/main/java/com/etesync/syncadapter/syncadapter/ContactsSyncManager.java b/app/src/main/java/com/etesync/syncadapter/syncadapter/ContactsSyncManager.java index a8e2917f..98b57c34 100644 --- a/app/src/main/java/com/etesync/syncadapter/syncadapter/ContactsSyncManager.java +++ b/app/src/main/java/com/etesync/syncadapter/syncadapter/ContactsSyncManager.java @@ -55,8 +55,6 @@ import okhttp3.ResponseBody; *

Synchronization manager for CardDAV collections; handles contacts and groups.

*/ public class ContactsSyncManager extends SyncManager { - protected static final int MAX_MULTIGET = 10; - final private ContentProviderClient provider; final private HttpUrl remote; diff --git a/app/src/main/java/com/etesync/syncadapter/syncadapter/SyncManager.java b/app/src/main/java/com/etesync/syncadapter/syncadapter/SyncManager.java index 01a57b0f..d76b2f22 100644 --- a/app/src/main/java/com/etesync/syncadapter/syncadapter/SyncManager.java +++ b/app/src/main/java/com/etesync/syncadapter/syncadapter/SyncManager.java @@ -53,6 +53,9 @@ import okhttp3.OkHttpClient; import static com.etesync.syncadapter.Constants.KEY_ACCOUNT; abstract public class SyncManager { + private static final int MAX_FETCH = 50; + private static final int MAX_PUSH = 30; + protected final NotificationHelper notificationManager; protected final CollectionInfo info; @@ -151,17 +154,19 @@ abstract public class SyncManager { App.log.info("Sync phase: " + context.getString(syncPhase)); prepareLocal(); - if (Thread.interrupted()) - throw new InterruptedException(); - syncPhase = R.string.sync_phase_fetch_entries; - App.log.info("Sync phase: " + context.getString(syncPhase)); - fetchEntries(); + do { + if (Thread.interrupted()) + throw new InterruptedException(); + syncPhase = R.string.sync_phase_fetch_entries; + App.log.info("Sync phase: " + context.getString(syncPhase)); + fetchEntries(); - if (Thread.interrupted()) - throw new InterruptedException(); - syncPhase = R.string.sync_phase_apply_remote_entries; - App.log.info("Sync phase: " + context.getString(syncPhase)); - applyRemoteEntries(); + if (Thread.interrupted()) + throw new InterruptedException(); + syncPhase = R.string.sync_phase_apply_remote_entries; + App.log.info("Sync phase: " + context.getString(syncPhase)); + applyRemoteEntries(); + } while (remoteEntries.size() > 0); /* Create journal entries out of local changes. */ if (Thread.interrupted()) @@ -282,7 +287,7 @@ abstract public class SyncManager { int count = data.count(EntryEntity.class).where(EntryEntity.JOURNAL.eq(getJournalEntity())).get().value(); if ((remoteCTag != null) && (count == 0)) { // If we are updating an existing installation with no saved journal, we need to add - remoteEntries = journal.list(crypto, null); + remoteEntries = journal.list(crypto, null, MAX_FETCH); int i = 0; for (JournalEntryManager.Entry entry : remoteEntries) { SyncEntry cEntry = SyncEntry.fromJournalEntry(crypto, entry); @@ -294,7 +299,7 @@ abstract public class SyncManager { } } } else { - remoteEntries = journal.list(crypto, remoteCTag); + remoteEntries = journal.list(crypto, remoteCTag, MAX_FETCH); } App.log.info("Fetched " + String.valueOf(remoteEntries.size()) + " entries"); @@ -324,7 +329,6 @@ abstract public class SyncManager { protected void pushEntries() throws Exceptions.HttpException, IOException, ContactsStorageException, CalendarStorageException { // upload dirty contacts - final int MAX_PUSH = 30; int pushed = 0; // FIXME: Deal with failure (someone else uploaded before we go here) try { diff --git a/app/src/test/java/com/etesync/syncadapter/journalmanager/ServiceTest.java b/app/src/test/java/com/etesync/syncadapter/journalmanager/ServiceTest.java index 78e114d6..54edeede 100644 --- a/app/src/test/java/com/etesync/syncadapter/journalmanager/ServiceTest.java +++ b/app/src/test/java/com/etesync/syncadapter/journalmanager/ServiceTest.java @@ -180,9 +180,9 @@ public class ServiceTest { entries.add(entry2); // Check last works: - entries = journalEntryManager.list(crypto, entry.getUid()); + entries = journalEntryManager.list(crypto, entry.getUid(), 0); assertEquals(entries.size(), 1); - entries = journalEntryManager.list(crypto, entry2.getUid()); + entries = journalEntryManager.list(crypto, entry2.getUid(), 0); assertEquals(entries.size(), 0); // Corrupt the journal and verify we catch it @@ -195,7 +195,7 @@ public class ServiceTest { try { caught = null; - journalEntryManager.list(crypto, null); + journalEntryManager.list(crypto, null, 0); } catch (Exceptions.IntegrityException e) { caught = e; }