[fix] Add a poor man's two-phase commit algorithm for syncback tasks.

Summary:
Sometimes things go very wrong when trying to syncback an action. For example, the worker window could crash, preventing us from marking the current task as failed. What's worse, another sync iteration could try to run the task which crashed, thinking it a new one. To prevent this, this diff adds a fourth syncback task state, `INPROGRESS`.

New syncback tasks are marked as `INPROGRESS` before being executed. When they complete we mark them as `SUCCEEDED/FAILED`. Stray `INPROGRESS` tasks are automatically marked as `FAILED` at the beginning of every sync iteration, to make sure we don't retry them again.

Test Plan: Tested manually.

Reviewers: evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D3635
This commit is contained in:
Karim Hamidou 2017-01-11 12:04:58 -08:00
parent 47f5880d18
commit 9136b0592b
2 changed files with 31 additions and 4 deletions

View file

@ -257,10 +257,33 @@ class SyncWorker {
return tasksToProcess
}
async _markInProgressTasksAsFailed() {
// We use a very limited type of two-phase commit: before we start
// running a syncback task, we mark it as "in progress". If something
// happens during the syncback (the worker window crashes, or the power
// goes down), the task won't succeed or fail.
// We absolutely never want to retry such a task, so we mark it as failed
// at the next sync iteration. We use this function for that.
const {SyncbackRequest} = this._db;
const inProgressTasks = await SyncbackRequest.findAll({
where: {status: 'INPROGRESS'},
});
for (const inProgress of inProgressTasks) {
inProgress.status = 'FAILED';
await inProgress.save();
}
}
async _runSyncbackTask(task) {
const syncbackRequest = task.syncbackRequestObject();
console.log(`🔃 📤 ${task.description()}`, syncbackRequest.props)
try {
// Before anything, mark the task as in progress. This allows
// us to not run the same task twice.
syncbackRequest.status = "INPROGRESS";
await syncbackRequest.save();
const responseJSON = await this._conn.runOperation(task);
syncbackRequest.status = "SUCCEEDED";
syncbackRequest.responseJSON = responseJSON || {};
@ -386,19 +409,23 @@ class SyncWorker {
yield this._account.update({syncError: null});
yield this._ensureConnection();
// Step 1: Run any available syncback tasks
// Step 1: Mark all "INPROGRESS" tasks as failed.
await this._markInProgressTasksAsFailed()
yield // Yield to allow interruption
// Step 2: Run any available syncback tasks
const tasks = yield this._getNewSyncbackTasks()
for (const task of tasks) {
await this._runSyncbackTask(task)
yield // Yield to allow interruption
}
// Step 2: Fetch the folder list. We need to run this before syncing folders
// Step 3: Fetch the folder list. We need to run this before syncing folders
// because we need folders to sync!
await this._runOperation(new FetchFolderList(this._account, this._logger))
yield // Yield to allow interruption
// Step 3: Sync each folder, sorted by inbox first
// Step 4: Sync each folder, sorted by inbox first
// TODO prioritize syncing all of inbox first if there's a ton of folders (e.g. imap
// accounts). If there are many folders, we would only sync the first n
// messages in the inbox and not go back to it until we've done the same for

View file

@ -16,7 +16,7 @@ module.exports = (sequelize, Sequelize) => {
return sequelize.define('syncbackRequest', {
type: Sequelize.STRING,
status: {
type: Sequelize.ENUM("NEW", "SUCCEEDED", "FAILED"),
type: Sequelize.ENUM("NEW", "INPROGRESS", "SUCCEEDED", "FAILED"),
defaultValue: "NEW",
allowNull: false,
},