Bug 1308337 - Pre: Don't throw away store and fetch exceptions as they're encountered r=nalexander
authorGrigory Kruglov <gkruglov@mozilla.com>
Fri, 26 May 2017 17:44:27 -0400
changeset 412575 7fb2a2562a8bf820f0969385477df99e19aceb8e
parent 412574 b129a46b714ed56ed7252b9b96e959762d3a4aea
child 412576 38894ad99464973a4be0963641569dfa4d96ecdd
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs1308337, 1362208
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1308337 - Pre: Don't throw away store and fetch exceptions as they're encountered r=nalexander We will need them later for telemetry reporting. For now we're just keeping the last exception which we encountered (which agrees with desktop's behaviour), and Bug 1362208 explores follow-up work to aggregate and count the exceptions as we see them. MozReview-Commit-ID: 8yKkZVGJZ9e
mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/SynchronizerSession.java
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/SynchronizerSession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/SynchronizerSession.java
@@ -70,18 +70,26 @@ implements RecordsChannelDelegate,
   // that a concurrently syncing client has uploaded.
   private long pendingATimestamp = -1;
   private long pendingBTimestamp = -1;
   private long storeEndATimestamp = -1;
   private long storeEndBTimestamp = -1;
   private boolean flowAToBCompleted = false;
   private boolean flowBToACompleted = false;
 
-  protected final AtomicInteger numInboundRecords = new AtomicInteger(-1);
-  protected final AtomicInteger numOutboundRecords = new AtomicInteger(-1);
+  private final AtomicInteger numInboundRecords = new AtomicInteger(-1);
+  private final AtomicInteger numInboundRecordsStored = new AtomicInteger(-1);
+  private final AtomicInteger numInboundRecordsFailed = new AtomicInteger(-1);
+  private final AtomicInteger numInboundRecordsReconciled = new AtomicInteger(-1);
+  private final AtomicInteger numOutboundRecords = new AtomicInteger(-1);
+  private final AtomicInteger numOutboundRecordsStored = new AtomicInteger(-1);
+  private final AtomicInteger numOutboundRecordsFailed = new AtomicInteger(-1);
+
+  private Exception fetchFailedCauseException;
+  private Exception storeFailedCauseException;
 
   /*
    * Public API: constructor, init, synchronize.
    */
   public SynchronizerSession(Synchronizer synchronizer, SynchronizerSessionDelegate delegate) {
     this.setSynchronizer(synchronizer);
     this.delegate = delegate;
   }
@@ -121,16 +129,32 @@ implements RecordsChannelDelegate,
    * Valid only after second flow has completed.
    *
    * @return number of records, or -1 if not valid.
    */
   public int getOutboundCount() {
     return numOutboundRecords.get();
   }
 
+  public int getOutboundCountStored() {
+    return numOutboundRecordsStored.get();
+  }
+
+  public int getOutboundCountFailed() {
+    return numOutboundRecordsFailed.get();
+  }
+
+  public Exception getFetchFailedCauseException() {
+    return fetchFailedCauseException;
+  }
+
+  public Exception getStoreFailedCauseException() {
+    return storeFailedCauseException;
+  }
+
   // These are accessed by `abort` and `synchronize`, both of which are synchronized.
   // Guarded by `this`.
   protected RecordsChannel channelAToB;
   protected RecordsChannel channelBToA;
 
   /**
    * Please don't call this until you've been notified with onInitialized.
    */
@@ -167,21 +191,26 @@ implements RecordsChannelDelegate,
       public void onFlowBeginFailed(RecordsChannel recordsChannel, Exception ex) {
         Logger.warn(LOG_TAG, "First RecordsChannel onFlowBeginFailed. Logging session error.", ex);
         session.delegate.onSynchronizeFailed(session, ex, "Failed to begin first flow.");
       }
 
       @Override
       public void onFlowFetchFailed(RecordsChannel recordsChannel, Exception ex) {
         Logger.warn(LOG_TAG, "First RecordsChannel onFlowFetchFailed. Logging remote fetch error.", ex);
+        fetchFailedCauseException = ex;
       }
 
       @Override
       public void onFlowStoreFailed(RecordsChannel recordsChannel, Exception ex, String recordGuid) {
         Logger.warn(LOG_TAG, "First RecordsChannel onFlowStoreFailed. Logging local store error.", ex);
+        // Currently we're just recording the very last exception which occurred. This is a reasonable
+        // approach, but ideally we'd want to categorize the exceptions and count them for the purposes
+        // of better telemetry. See Bug 1362208.
+        storeFailedCauseException = ex;
       }
 
       @Override
       public void onFlowFinishFailed(RecordsChannel recordsChannel, Exception ex) {
         Logger.warn(LOG_TAG, "First RecordsChannel onFlowFinishedFailed. Logging session error.", ex);
         session.delegate.onSynchronizeFailed(session, ex, "Failed to finish first flow.");
       }
     };