[client-sync] Use ExponentialBackoffScheduler for sync loop retries

Summary:
This is mostly to clean up the logic here a little bit.

Specifically:

- The backoff when encountering retryable errors is now truly exponential
- When encountering a permanent error, we back off for a minute
- If we don't encounter consecutive RetryableErrors, we clear the exponential backoff. Previously, we would just clear it when sync completed without errors.

Test Plan: manual

Reviewers: khamidou, spang, evan, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D4086
This commit is contained in:
Juan Tejada 2017-03-02 14:47:34 -08:00
parent 9a01aa7bcb
commit 225613565a

View file

@ -1,27 +1,31 @@
const _ = require('underscore') import _ from 'underscore'
const { import {
IMAPErrors, IMAPErrors,
IMAPConnectionPool,
SendmailClient, SendmailClient,
MetricsReporter, MetricsReporter,
} = require('isomorphic-core'); IMAPConnectionPool,
const { ExponentialBackoffScheduler,
} from 'isomorphic-core';
import {
Actions, Actions,
Account,
APIError, APIError,
NylasAPI, NylasAPI,
N1CloudAPI, N1CloudAPI,
IdentityStore, IdentityStore,
NylasAPIRequest, NylasAPIRequest,
BatteryStatusManager, BatteryStatusManager,
Account: {SYNC_STATE_RUNNING, SYNC_STATE_AUTH_FAILED, SYNC_STATE_ERROR}, } from 'nylas-exports'
} = require('nylas-exports') import Interruptible from '../shared/interruptible'
const Interruptible = require('../shared/interruptible') import SyncTaskFactory from './sync-task-factory';
const SyncTaskFactory = require('./sync-task-factory'); import SyncbackTaskRunner from './syncback-task-runner'
const SyncbackTaskRunner = require('./syncback-task-runner').default; import LocalSyncDeltaEmitter from './local-sync-delta-emitter'
const LocalSyncDeltaEmitter = require('./local-sync-delta-emitter').default
const {SYNC_STATE_RUNNING, SYNC_STATE_AUTH_FAILED, SYNC_STATE_ERROR} = Account
const AC_SYNC_LOOP_INTERVAL_MS = 10 * 1000 // 10 sec const AC_SYNC_LOOP_INTERVAL_MS = 10 * 1000 // 10 sec
const BATTERY_SYNC_LOOP_INTERVAL_MS = 5 * 60 * 1000 // 5 min const BATTERY_SYNC_LOOP_INTERVAL_MS = 5 * 60 * 1000 // 5 min
const MAX_SYNC_BACKOFF_MS = 5 * 60 * 1000 // 5 min
const PERMANENT_ERROR_RETRY_BACKOFF_MS = 60 * 1000 // 1 min
class SyncWorker { class SyncWorker {
constructor(account, db, parentManager) { constructor(account, db, parentManager) {
@ -43,12 +47,16 @@ class SyncWorker {
this._stopped = false this._stopped = false
this._destroyed = false this._destroyed = false
this._shouldIgnoreInboxFlagUpdates = false this._shouldIgnoreInboxFlagUpdates = false
this._numRetries = 0;
this._numTimeoutErrors = 0; this._numTimeoutErrors = 0;
this._requireTokenRefresh = false this._requireTokenRefresh = false
this._batchProcessedUids = new Map(); this._batchProcessedUids = new Map();
this._latestOpenTimesByFolder = new Map(); this._latestOpenTimesByFolder = new Map();
this._retryScheduler = new ExponentialBackoffScheduler({
baseDelay: 15 * 1000,
maxDelay: MAX_SYNC_BACKOFF_MS,
})
this._syncTimer = setTimeout(() => { this._syncTimer = setTimeout(() => {
// TODO this is currently a hack to keep N1's account in sync and notify of // TODO this is currently a hack to keep N1's account in sync and notify of
// sync errors. This should go away when we merge the databases // sync errors. This should go away when we merge the databases
@ -312,9 +320,12 @@ class SyncWorker {
// If so, we don't want to save the error to the account, which will cause // If so, we don't want to save the error to the account, which will cause
// a red box to show up. // a red box to show up.
if (error instanceof IMAPErrors.RetryableError) { if (error instanceof IMAPErrors.RetryableError) {
this._numRetries += 1; this._retryScheduler.nextDelay()
return return
} }
// If we don't encounter consecutive RetryableErrors, reset the exponential
// backoff
this._retryScheduler.reset()
// Update account error state // Update account error state
const errorJSON = error.toJSON() const errorJSON = error.toJSON()
@ -366,22 +377,11 @@ class SyncWorker {
let interval; let interval;
if (error != null) { if (error != null) {
if (error instanceof IMAPErrors.RetryableError) { if (error instanceof IMAPErrors.RetryableError) {
// We do not want to retry over and over again when we get interval = this._retryScheduler.currentDelay();
// network/retryable errors, for two reasons:
// 1. most errors don't resolve immediately
// 2. we don't want to be hammering the server in a synchronized way.
const randomElement = (Math.floor(Math.random() * 20) + 1) * 1000;
const exponentialDuration = 15000 * this._numRetries + randomElement;
interval = Math.min(exponentialDuration, 120000 + randomElement);
} else { } else {
// Encountered a permanent error interval = PERMANENT_ERROR_RETRY_BACKOFF_MS;
// In this case we could do something fancier, but for now, just retry
// with the normal sync loop interval
interval = AC_SYNC_LOOP_INTERVAL_MS;
} }
} else { } else {
// Continue syncing immediately if initial sync isn't done, or if the loop was
// interrupted or a sync was requested
const shouldSyncImmediately = ( const shouldSyncImmediately = (
moreToSync || moreToSync ||
this._interrupted || this._interrupted ||
@ -526,10 +526,11 @@ class SyncWorker {
}) })
}, },
}); });
await this._cleanupOrphanMessages(); await this._cleanupOrphanMessages();
await this._onSyncDidComplete(); await this._onSyncDidComplete();
this._numRetries = 0;
this._numTimeoutErrors = 0; this._numTimeoutErrors = 0;
this._retryScheduler.reset()
} catch (err) { } catch (err) {
error = err error = err
await this._onSyncError(error); await this._onSyncError(error);