From dc5b52e557165f663b05f8e233125102c997e004 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Tue, 1 Nov 2016 18:52:18 -0700 Subject: [PATCH] Bug 1291821 - Use sync deadline to decide of batching downloader should proceed r=rnewman MozReview-Commit-ID: IDgIj9lBt61 --HG-- extra : rebase_source : a3d1773abb50748631e28c0aa14797b17b857def --- .../ConstrainedServer11Repository.java | 2 ++ .../sync/repositories/Server11Repository.java | 13 ++++++++++++- .../repositories/Server11RepositorySession.java | 1 + .../downloaders/BatchingDownloader.java | 17 +++++++++++++++++ .../AndroidBrowserBookmarksServerSyncStage.java | 1 + .../AndroidBrowserHistoryServerSyncStage.java | 1 + .../gecko/sync/stage/ServerSyncStage.java | 1 + 7 files changed, 35 insertions(+), 1 deletion(-) diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/ConstrainedServer11Repository.java b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/ConstrainedServer11Repository.java index 4387d64d6364..3713b764fd20 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/ConstrainedServer11Repository.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/ConstrainedServer11Repository.java @@ -24,6 +24,7 @@ public class ConstrainedServer11Repository extends Server11Repository { public ConstrainedServer11Repository( String collection, + long syncDeadline, String storageURL, AuthHeaderProvider authHeaderProvider, InfoCollections infoCollections, @@ -33,6 +34,7 @@ public class ConstrainedServer11Repository extends Server11Repository { boolean allowMultipleBatches) throws URISyntaxException { super( collection, + syncDeadline, storageURL, authHeaderProvider, infoCollections, diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11Repository.java b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11Repository.java index e04ce486e90c..917416075c15 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11Repository.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11Repository.java @@ -25,7 +25,7 @@ import android.support.annotation.Nullable; public class Server11Repository extends Repository { public final AuthHeaderProvider authHeaderProvider; - /* package-local */ final long syncDeadline; + private final long syncDeadlineMillis; /* package-local */ final URI collectionURI; protected final String collection; @@ -46,6 +46,7 @@ public class Server11Repository extends Repository { */ public Server11Repository( @NonNull String collection, + long syncDeadlineMillis, @NonNull String storageURL, AuthHeaderProvider authHeaderProvider, @NonNull InfoCollections infoCollections, @@ -60,6 +61,7 @@ public class Server11Repository extends Repository { throw new IllegalArgumentException("infoCollections must not be null"); } this.collection = collection; + this.syncDeadlineMillis = syncDeadlineMillis; this.collectionURI = new URI(storageURL + (storageURL.endsWith("/") ? collection : "/" + collection)); this.authHeaderProvider = authHeaderProvider; this.infoCollections = infoCollections; @@ -100,4 +102,13 @@ public class Server11Repository extends Repository { public boolean getAllowMultipleBatches() { return true; } + + /** + * A point in time by which this repository's session must complete fetch and store operations. + * Particularly pertinent for batching downloads performed by the session (should we fetch + * another batch?) and buffered repositories (do we have enough time to merge what we've downloaded?). + */ + public long getSyncDeadline() { + return syncDeadlineMillis; + } } diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11RepositorySession.java b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11RepositorySession.java index 0680850f885c..274f838f8708 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11RepositorySession.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11RepositorySession.java @@ -112,6 +112,7 @@ public class Server11RepositorySession extends RepositorySession { return new BatchingDownloader( serverRepositorySession.serverRepository.authHeaderProvider, Uri.parse(serverRepositorySession.serverRepository.collectionURI().toString()), + serverRepositorySession.serverRepository.getSyncDeadline(), serverRepositorySession.serverRepository.getAllowMultipleBatches(), serverRepositorySession); } diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloader.java b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloader.java index bafc5315d32f..13358b1e87be 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloader.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloader.java @@ -58,6 +58,7 @@ public class BatchingDownloader { private final RepositorySession repositorySession; private final DelayedWorkTracker workTracker = new DelayedWorkTracker(); private final Uri baseCollectionUri; + private final long fetchDeadline; private final boolean allowMultipleBatches; /* package-local */ final AuthHeaderProvider authHeaderProvider; @@ -70,12 +71,14 @@ public class BatchingDownloader { public BatchingDownloader( AuthHeaderProvider authHeaderProvider, Uri baseCollectionUri, + long fetchDeadline, boolean allowMultipleBatches, RepositorySession repositorySession) { this.repositorySession = repositorySession; this.authHeaderProvider = authHeaderProvider; this.baseCollectionUri = baseCollectionUri; this.allowMultipleBatches = allowMultipleBatches; + this.fetchDeadline = fetchDeadline; } @VisibleForTesting @@ -217,6 +220,12 @@ public class BatchingDownloader { } }); + // Should we proceed, however? Do we have enough time? + if (!mayProceedWithBatching(fetchDeadline)) { + this.abort(fetchRecordsDelegate, new Exception("Not enough time to complete next batch")); + return; + } + // Create and execute new batch request. try { final SyncStorageCollectionRequest newRequest = makeSyncStorageCollectionRequest(newer, @@ -293,6 +302,14 @@ public class BatchingDownloader { } }); } + + private static boolean mayProceedWithBatching(long deadline) { + // For simplicity, allow batching to proceed if there's at least a minute left for the sync. + // This should be enough to fetch and process records in the batch. + final long timeLeft = deadline - SystemClock.elapsedRealtime(); + return timeLeft > TimeUnit.MINUTES.toMillis(1); + } + @VisibleForTesting public static URI buildCollectionURI(Uri baseCollectionUri, boolean full, long newer, long limit, String sort, String ids, String offset) throws URISyntaxException { Uri.Builder uriBuilder = baseCollectionUri.buildUpon(); diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserBookmarksServerSyncStage.java b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserBookmarksServerSyncStage.java index b4e412979b26..c95d7c31ed7d 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserBookmarksServerSyncStage.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserBookmarksServerSyncStage.java @@ -41,6 +41,7 @@ public class AndroidBrowserBookmarksServerSyncStage extends ServerSyncStage { protected Repository getRemoteRepository() throws URISyntaxException { return new ConstrainedServer11Repository( getCollection(), + session.getSyncDeadline(), session.config.storageURL(), session.getAuthHeaderProvider(), session.config.infoCollections, diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserHistoryServerSyncStage.java b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserHistoryServerSyncStage.java index 08b0964fce3d..28cbe3a56a3a 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserHistoryServerSyncStage.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserHistoryServerSyncStage.java @@ -46,6 +46,7 @@ public class AndroidBrowserHistoryServerSyncStage extends ServerSyncStage { protected Repository getRemoteRepository() throws URISyntaxException { return new ConstrainedServer11Repository( getCollection(), + session.getSyncDeadline(), session.config.storageURL(), session.getAuthHeaderProvider(), session.config.infoCollections, diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/ServerSyncStage.java b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/ServerSyncStage.java index 8cda3747c620..9df30d6c0bf3 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/ServerSyncStage.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/ServerSyncStage.java @@ -144,6 +144,7 @@ public abstract class ServerSyncStage extends AbstractSessionManagingSyncStage i protected Repository getRemoteRepository() throws URISyntaxException { String collection = getCollection(); return new Server11Repository(collection, + session.getSyncDeadline(), session.config.storageURL(), session.getAuthHeaderProvider(), session.config.infoCollections,