Bug 1291821 - Use sync deadline to decide of batching downloader should proceed r=rnewman
authorGrisha Kruglov <gkruglov@mozilla.com>
Tue, 01 Nov 2016 18:52:18 -0700
changeset 373896 3a43ecf3c62573024cd9b9d45d2edfc9c9e8d319
parent 373895 50608e0550054650b192bc451c3aeab6e6a0d5e6
child 373897 d61efb5192df344972d4c5c0f21d4d1c04459441
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1291821
milestone54.0a1
Bug 1291821 - Use sync deadline to decide of batching downloader should proceed r=rnewman MozReview-Commit-ID: IDgIj9lBt61
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/ConstrainedServer11Repository.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11Repository.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server11RepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/downloaders/BatchingDownloader.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserBookmarksServerSyncStage.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/AndroidBrowserHistoryServerSyncStage.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/stage/ServerSyncStage.java
--- 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
@@ -19,25 +19,27 @@ import org.mozilla.gecko.sync.net.AuthHe
  */
 public class ConstrainedServer11Repository extends Server11Repository {
   private final String sortOrder;
   private final long batchLimit;
   private final boolean allowMultipleBatches;
 
   public ConstrainedServer11Repository(
           String collection,
+          long syncDeadline,
           String storageURL,
           AuthHeaderProvider authHeaderProvider,
           InfoCollections infoCollections,
           InfoConfiguration infoConfiguration,
           long batchLimit,
           String sort,
           boolean allowMultipleBatches) throws URISyntaxException {
     super(
             collection,
+            syncDeadline,
             storageURL,
             authHeaderProvider,
             infoCollections,
             infoConfiguration
     );
     this.batchLimit = batchLimit;
     this.sortOrder  = sort;
     this.allowMultipleBatches = allowMultipleBatches;
--- 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
@@ -20,17 +20,17 @@ import android.support.annotation.Nullab
  * A Server11Repository implements fetching and storing against the Sync 1.5 API.
  * It doesn't do crypto: that's the job of the middleware.
  *
  * @author rnewman
  */
 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;
   protected final InfoCollections infoCollections;
 
   private final InfoConfiguration infoConfiguration;
   private final static String DEFAULT_SORT_ORDER = "oldest";
   private final static long DEFAULT_BATCH_LIMIT = 100;
@@ -41,30 +41,32 @@ public class Server11Repository extends 
    * @param collection name.
    * @param storageURL full URL to storage endpoint.
    * @param authHeaderProvider to use in requests; may be null.
    * @param infoCollections instance; must not be null.
    * @throws URISyntaxException
    */
   public Server11Repository(
           @NonNull String collection,
+          long syncDeadlineMillis,
           @NonNull String storageURL,
           AuthHeaderProvider authHeaderProvider,
           @NonNull InfoCollections infoCollections,
           @NonNull InfoConfiguration infoConfiguration) throws URISyntaxException {
     if (collection == null) {
       throw new IllegalArgumentException("collection must not be null");
     }
     if (storageURL == null) {
       throw new IllegalArgumentException("storageURL must not be null");
     }
     if (infoCollections == null) {
       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;
     this.infoConfiguration = infoConfiguration;
   }
 
   @Override
   public void createSession(RepositorySessionCreationDelegate delegate,
@@ -95,9 +97,18 @@ public class Server11Repository extends 
 
   public Long getBatchLimit() {
     return DEFAULT_BATCH_LIMIT;
   }
 
   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;
+  }
 }
--- 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
@@ -107,12 +107,13 @@ public class Server11RepositorySession e
   public boolean dataAvailable() {
     return serverRepository.updateNeeded(getLastSyncTimestamp());
   }
 
   protected static BatchingDownloader initializeDownloader(final Server11RepositorySession serverRepositorySession) {
     return new BatchingDownloader(
             serverRepositorySession.serverRepository.authHeaderProvider,
             Uri.parse(serverRepositorySession.serverRepository.collectionURI().toString()),
+            serverRepositorySession.serverRepository.getSyncDeadline(),
             serverRepositorySession.serverRepository.getAllowMultipleBatches(),
             serverRepositorySession);
   }
 }
--- 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
@@ -53,34 +53,37 @@ import java.util.concurrent.TimeUnit;
  */
 public class BatchingDownloader {
     public static final String LOG_TAG = "BatchingDownloader";
     private static final String DEFAULT_SORT_ORDER = "index";
 
     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;
 
     // Used to track outstanding requests, so that we can abort them as needed.
     @VisibleForTesting
     protected final Set<SyncStorageCollectionRequest> pending = Collections.synchronizedSet(new HashSet<SyncStorageCollectionRequest>());
     /* @GuardedBy("this") */ private String lastModified;
 
     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
     protected static String flattenIDs(String[] guids) {
         // Consider using Utils.toDelimitedString if and when the signature changes
         // to Collection<String> guids.
         if (guids.length == 0) {
             return "";
@@ -212,16 +215,22 @@ public class BatchingDownloader {
         this.workTracker.delayWorkItem(new Runnable() {
             @Override
             public void run() {
                 Logger.debug(LOG_TAG, "Running onBatchCompleted.");
                 fetchRecordsDelegate.onBatchCompleted();
             }
         });
 
+        // 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,
                     limit, full, sort, ids, offset);
             this.fetchWithParameters(newer, limit, full, sort, ids, newRequest, fetchRecordsDelegate);
         } catch (final URISyntaxException | UnsupportedEncodingException e) {
             this.workTracker.delayWorkItem(new Runnable() {
                 @Override
@@ -288,16 +297,24 @@ public class BatchingDownloader {
         this.workTracker.delayWorkItem(new Runnable() {
             @Override
             public void run() {
                 Logger.debug(LOG_TAG, "Delayed onFetchCompleted running.");
                 delegate.onFetchFailed(exception);
             }
         });
     }
+
+    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();
 
         if (full) {
             uriBuilder.appendQueryParameter("full", "1");
         }
 
--- 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
@@ -36,16 +36,17 @@ public class AndroidBrowserBookmarksServ
   public Integer getStorageVersion() {
     return VersionConstants.BOOKMARKS_ENGINE_VERSION;
   }
 
   @Override
   protected Repository getRemoteRepository() throws URISyntaxException {
     return new ConstrainedServer11Repository(
             getCollection(),
+            session.getSyncDeadline(),
             session.config.storageURL(),
             session.getAuthHeaderProvider(),
             session.config.infoCollections,
             session.config.infoConfiguration,
             BOOKMARKS_BATCH_LIMIT,
             BOOKMARKS_SORT,
             true /* allow multiple batches */);
   }
--- 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
@@ -41,16 +41,17 @@ public class AndroidBrowserHistoryServer
   protected Repository getLocalRepository() {
     return new AndroidBrowserHistoryRepository();
   }
 
   @Override
   protected Repository getRemoteRepository() throws URISyntaxException {
     return new ConstrainedServer11Repository(
             getCollection(),
+            session.getSyncDeadline(),
             session.config.storageURL(),
             session.getAuthHeaderProvider(),
             session.config.infoCollections,
             session.config.infoConfiguration,
             HISTORY_BATCH_LIMIT,
             HISTORY_SORT,
             true /* allow multiple batches */);
   }
--- 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
@@ -139,16 +139,17 @@ public abstract class ServerSyncStage ex
   protected abstract String getEngineName();
   protected abstract Repository getLocalRepository();
   protected abstract RecordFactory getRecordFactory();
 
   // Override this in subclasses.
   protected Repository getRemoteRepository() throws URISyntaxException {
     String collection = getCollection();
     return new Server11Repository(collection,
+                                  session.getSyncDeadline(),
                                   session.config.storageURL(),
                                   session.getAuthHeaderProvider(),
                                   session.config.infoCollections,
                                   session.config.infoConfiguration);
   }
 
   /**
    * Return a Crypto5Middleware-wrapped Server11Repository.