From 7e99d0aca03166a2ee3ceda136f32262d80f4779 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 15 Oct 2014 09:23:11 -0700 Subject: [PATCH 1/7] Callback with error instead of throwing in redis driver --- lib/redisdriver.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/redisdriver.js b/lib/redisdriver.js index 967bb837a..3c1d1802e 100644 --- a/lib/redisdriver.js +++ b/lib/redisdriver.js @@ -172,7 +172,7 @@ RedisDriver.prototype.atomicSubmit = function(cName, docName, opData, options, c // In this case, we should write a no-op ramp to the snapshot // version, followed by a delete & a create to fill in the missing // ops. - throw Error('Missing oplog for ' + cName + ' ' + docName); + return callback('Missing oplog for ' + cName + ' ' + docName); } self._redisSubmitScript(cName, docName, opData, dirtyData, version, callbackWrapper); }); @@ -321,7 +321,8 @@ RedisDriver.prototype._oplogGetOps = function(cName, docName, from, to, callback var self = this; this.oplog.getOps(cName, docName, from, to, function(err, ops) { if (err) return callback(err); - if (ops.length && ops[0].v !== from) throw Error('Oplog is returning incorrect ops'); + if (ops.length && ops[0].v !== from) + return callback('Oplog is returning incorrect ops'); for (var i = 0; i < ops.length; i++) { ops[i].v = from++; From 48c8bb16a24df6f6e4e16bc7515c3ff28444ee13 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Fri, 17 Oct 2014 18:22:29 -0700 Subject: [PATCH 2/7] Callback with an error when ops are missing from the oplog --- lib/index.js | 5 +++++ test/test.coffee | 30 ++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/lib/index.js b/lib/index.js index 065c52b65..cb2b6c5c5 100644 --- a/lib/index.js +++ b/lib/index.js @@ -260,6 +260,11 @@ Livedb.prototype.submit = function(cName, docName, opData, options, callback) { + "Please file an issue if you can recreate this state reliably."); return callback('Internal data corruption - cannot submit'); } + + // Check if the first op we received wasn't the one we requested. + // This can occur if the ops were dropped from the oplog (e.g. expired). + if (from !== snapshot.v && (ops.length === 0 || ops[0].v !== from)) + return callback('Missing operations'); for (var i = 0; i < ops.length; i++) { var op = ops[i]; diff --git a/test/test.coffee b/test/test.coffee index a23772742..37add6210 100644 --- a/test/test.coffee +++ b/test/test.coffee @@ -69,6 +69,36 @@ describe 'livedb', -> throw new Error err if err assert.equal m.language, null done() + + it 'errors when oplog is missing all operations', (done) -> @create => + @collection.submit @docName, {v:1, op:['test']}, (err, v) => + throw new Error err if err + + getOps = @client.driver.getOps + @client.driver.getOps = (cName, docName, from, to, fn) -> + fn null, [] + + @collection.submit @docName, {v:1, op:['test']}, (err, v) => + assert.equal err, 'Missing operations' + @client.driver.getOps = getOps + done() + + it 'errors when oplog is missing some operations', (done) -> @create => + @collection.submit @docName, {v:1, op:['test']}, (err, v) => + throw new Error err if err + @collection.submit @docName, {v:2, op:['test 2']}, (err, v) => + throw new Error err if err + + getOps = @client.driver.getOps + @client.driver.getOps = (cName, docName, from, to, fn) => + getOps.call @client.driver, cName, docName, from, to, (err, ops) -> + throw new Error err if err + fn null, ops.slice 1 + + @collection.submit @docName, {v:1, op:['test']}, (err, v) => + assert.equal err, 'Missing operations' + @client.driver.getOps = getOps + done() it 'can modify a document', (done) -> @create => @collection.submit @docName, v:1, op:['hi'], (err, v) => From 74556cfb6d1704729f9d94c549f83b0675621917 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sat, 18 Oct 2014 11:44:42 -0700 Subject: [PATCH 3/7] Matching error message in redis driver --- lib/redisdriver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/redisdriver.js b/lib/redisdriver.js index 3c1d1802e..c77264787 100644 --- a/lib/redisdriver.js +++ b/lib/redisdriver.js @@ -322,7 +322,7 @@ RedisDriver.prototype._oplogGetOps = function(cName, docName, from, to, callback this.oplog.getOps(cName, docName, from, to, function(err, ops) { if (err) return callback(err); if (ops.length && ops[0].v !== from) - return callback('Oplog is returning incorrect ops'); + return callback('Missing operations'); for (var i = 0; i < ops.length; i++) { ops[i].v = from++; From b43b256df73508d3974b9b90ebb19710a14f0af9 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sat, 18 Oct 2014 12:05:49 -0700 Subject: [PATCH 4/7] Error in getOps if ops are missing --- lib/index.js | 5 +++++ test/test.coffee | 21 ++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/lib/index.js b/lib/index.js index cb2b6c5c5..d1558d424 100644 --- a/lib/index.js +++ b/lib/index.js @@ -187,6 +187,11 @@ Livedb.prototype.getOps = function(cName, docName, from, to, callback) { this.driver.getOps(c, docName, from, to, function(err, ops) { if (self.sdc) self.sdc.timing('livedb.getOps', Date.now() - start); + + // If we got results, check that we got the right ops. + // Sometimes, if ops have expired from the oplog, there might be some missing. + if (ops.length > 0 && ops[0].v !== from) + return callback('Missing operations'); // Interestingly, this will filter ops for other types as if they were the projected type. This // is a bug, but it shouldn't cause any problems for now. I'll have to revisit this diff --git a/test/test.coffee b/test/test.coffee index 37add6210..706e5a08b 100644 --- a/test/test.coffee +++ b/test/test.coffee @@ -405,11 +405,22 @@ describe 'livedb', -> assert.deepEqual stripTs(ops), [{create:{type:textType.uri, data:''}, v:0, m:{}}, {op:['hi'], v:1, m:{}}] done() - - - it 'errors if ops are missing from the snapshotdb and oplogs' - - + it 'errors if ops are missing from the snapshotdb and oplogs', (done) -> @create => + @collection.submit @docName, {v:1, op:['test']}, (err, v) => + throw new Error err if err + @collection.submit @docName, {v:1, op:['test2']}, (err, v) => + throw new Error err if err + + getOps = @client.driver.getOps + @client.driver.getOps = (cName, docName, from, to, fn) => + getOps.call @client.driver, cName, docName, from, to, (err, ops) -> + throw new Error err if err + fn null, ops.slice 1 + + @collection.getOps @docName, 0, (err, ops) => + assert.equal err, 'Missing operations' + @client.driver.getOps = getOps + done() it 'works with separate clients', (done) -> @create => return done() unless @driver.distributed From 9f6963085bcd38ad9f573cbe23b11f014418f912 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Mon, 3 Nov 2014 20:16:04 -0800 Subject: [PATCH 5/7] Update getOps test for behavior change Requested ranges are checked against the returned ops now by livedb-mongo, so an error will be returned if ops are missing. --- test/oplog.coffee | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/oplog.coffee b/test/oplog.coffee index d6b3c39fa..8776c230a 100644 --- a/test/oplog.coffee +++ b/test/oplog.coffee @@ -98,15 +98,10 @@ module.exports = (create) -> check = (error, ops) -> throw new Error error if error assert.deepEqual ops, [] - done() if ++num is 7 + done() if ++num is 2 @db.getOps @cName, @docName, 0, 0, check - @db.getOps @cName, @docName, 0, 1, check - @db.getOps @cName, @docName, 0, 10, check @db.getOps @cName, @docName, 0, null, check - @db.getOps @cName, @docName, 10, 10, check - @db.getOps @cName, @docName, 10, 11, check - @db.getOps @cName, @docName, 10, null, check it 'returns ops', (done) -> num = 0 From 0489c7e38cd9da3a0f733da6a2f9d7e8810e3cb9 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Mon, 9 Feb 2015 10:44:20 -0800 Subject: [PATCH 6/7] Fix case where ops is null --- lib/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/index.js b/lib/index.js index 3fb6e90a8..faf208dbf 100644 --- a/lib/index.js +++ b/lib/index.js @@ -216,7 +216,7 @@ Livedb.prototype.getOps = function(cName, docName, from, to, callback) { // If we got results, check that we got the right ops. // Sometimes, if ops have expired from the oplog, there might be some missing. - if (ops.length > 0 && ops[0].v !== from) + if (ops && ops.length > 0 && ops[0].v !== from) return callback('Missing operations'); // Interestingly, this will filter ops for other types as if they were the projected type. This From d9be12e94ebfa8654a36771d21d681b6583e3913 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Mon, 9 Feb 2015 10:48:50 -0800 Subject: [PATCH 7/7] Fix merge --- lib/index.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/index.js b/lib/index.js index faf208dbf..a71aec70d 100644 --- a/lib/index.js +++ b/lib/index.js @@ -335,6 +335,14 @@ Livedb.prototype._trySubmit = function(submitData) { self._resumeSubmitting(opData.src); return; } + + // Check if the first op we received wasn't the one we requested. + // This can occur if the ops were dropped from the oplog (e.g. expired). + if (from !== snapshot.v && (ops.length === 0 || ops[0].v !== from)) { + submitData.callback('Missing operations'); + self._resumeSubmitting(opData.src); + return; + } for (var i = 0; i < ops.length; i++) { var op = ops[i]; @@ -347,12 +355,7 @@ Livedb.prototype._trySubmit = function(submitData) { self._resumeSubmitting(opData.src); return; } - - // Check if the first op we received wasn't the one we requested. - // This can occur if the ops were dropped from the oplog (e.g. expired). - if (from !== snapshot.v && (ops.length === 0 || ops[0].v !== from)) - return callback('Missing operations'); - + // Bring both the op and the snapshot up to date. At least one of // these two conditionals should be true. if (snapshot.v === op.v) {