From a23ddf31c0c0e8937f1b37f63e2f8b8599b45d74 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 29 Jan 2016 12:36:03 +0000 Subject: [PATCH] allow packing of temporary ops --- .../app/coffee/PackManager.coffee | 13 +- .../PackManager/PackManagerTests.coffee | 180 +++++++++++++++++- 2 files changed, 183 insertions(+), 10 deletions(-) diff --git a/services/track-changes/app/coffee/PackManager.coffee b/services/track-changes/app/coffee/PackManager.coffee index 7d39c43c8d..29ec6fa584 100644 --- a/services/track-changes/app/coffee/PackManager.coffee +++ b/services/track-changes/app/coffee/PackManager.coffee @@ -278,17 +278,20 @@ module.exports = PackManager = if d.pack? top = null return - # skip temporary ops (we could pack these into temporary packs in future) - if d.expiresAt? - top = null - return sz = BSON.calculateObjectSize(d) - if top? && top.pack.length < PackManager.MAX_COUNT && top.sz + sz < PackManager.MAX_SIZE + # decide if this doc can be added to the current pack + validLength = top? && (top.pack.length < PackManager.MAX_COUNT) + validSize = top? && (top.sz + sz < PackManager.MAX_SIZE) + bothPermanent = top? && (top.expiresAt? is false) && (d.expiresAt? is false) + bothTemporary = top? && (top.expiresAt? is true) && (d.expiresAt? is true) + within1Day = bothTemporary && (d.meta.start_ts - top.meta.start_ts < 24 * 3600 * 1000) + if top? && validLength && validSize && (bothPermanent || (bothTemporary && within1Day)) top.pack = top.pack.concat {v: d.v, meta: d.meta, op: d.op, _id: d._id} top.sz += sz top.n += 1 top.v_end = d.v top.meta.end_ts = d.meta.end_ts + top.expiresAt = d.expiresAt if top.expiresAt? return else # create a new pack diff --git a/services/track-changes/test/unit/coffee/PackManager/PackManagerTests.coffee b/services/track-changes/test/unit/coffee/PackManager/PackManagerTests.coffee index e92bd09313..9ac4e5b96b 100644 --- a/services/track-changes/test/unit/coffee/PackManager/PackManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/PackManager/PackManagerTests.coffee @@ -1,5 +1,6 @@ sinon = require('sinon') chai = require('chai') +assert = require('chai').assert should = chai.should() expect = chai.expect modulePath = "../../../../app/js/PackManager.js" @@ -7,6 +8,7 @@ SandboxedModule = require('sandboxed-module') {ObjectId} = require("mongojs") bson = require("bson") BSON = new bson.BSONPure() +_ = require("underscore") tk = require "timekeeper" @@ -232,15 +234,183 @@ describe "PackManager", -> @callback.called.should.equal true describe "convertDocsToPacks", -> - describe "with several small packs", -> + describe "with several small ops", -> beforeEach -> @ops = [ - { op: "op-3", meta: "meta-3", v: 3}, - { op: "op-4", meta: "meta-4", v: 4} + { _id: "3", op: "op-3", meta: {start_ts: "ts3_s", end_ts: "ts3_e", user_id: "u3"}, v: 3}, + { _id: "4", op: "op-4", meta: {start_ts: "ts4_s", end_ts: "ts4_e", user_id: "u4"}, v: 4}, ] @PackManager.convertDocsToPacks @ops, @callback -# it "should create a single pack", -> -# @callback. + + it "should create a single pack", (done) -> + @PackManager.convertDocsToPacks @ops, (err, packs) => + assert.deepEqual packs, [ { + pack: [ + { _id: "3", op: "op-3", meta: {start_ts: "ts3_s", end_ts: "ts3_e", user_id: "u3"}, v: 3}, + { _id: "4", op: "op-4", meta: {start_ts: "ts4_s", end_ts: "ts4_e", user_id: "u4"}, v: 4}, + ] + v: 3 + v_end: 4 + meta: {start_ts: "ts3_s", end_ts: "ts4_e"} + n: 2 + sz: 202 + }] + done() + it "should call the callback", -> @callback.called.should.equal true + describe "with many small ops", -> + beforeEach -> + @ops = ({ _id: "#{n}", op: "op-#{n}", meta: {start_ts: "ts#{n}_s", end_ts: "ts#{n}_e", user_id: "u#{n}"}, v: n} for n in [0...1024]) + @PackManager.convertDocsToPacks @ops, @callback + + it "should create two packs", (done) -> + @PackManager.convertDocsToPacks @ops, (err, packs) => + assert.deepEqual packs, [ { + pack: @ops[0...512] + v: 0 + v_end: 511 + meta: {start_ts: "ts0_s", end_ts: "ts511_e"} + n: 512 + sz: 56282 + }, { + pack: @ops[512...1024] + v: 512 + v_end: 1023 + meta: {start_ts: "ts512_s", end_ts: "ts1023_e"} + n: 512 + sz: 56952 + }] + done() + + it "should call the callback", -> + @callback.called.should.equal true + + describe "with many temporary ops", -> + beforeEach -> + @ops = ({ _id: "#{n}", op: "op-#{n}", meta: {start_ts: n, end_ts: n+1, user_id: "u#{n}"}, v: n, expiresAt: n+24*3600*1000 } for n in [0...1024]) + @PackManager.convertDocsToPacks @ops, @callback + + it "should create two packs", (done) -> + @PackManager.convertDocsToPacks @ops, (err, packs) => + assert.deepEqual packs, [ { + pack: (_.omit(op, "expiresAt") for op in @ops[0...512]) + v: 0 + v_end: 511 + meta: {start_ts: 0 , end_ts: 512} + n: 512 + sz: 55990 + expiresAt: @ops[511].expiresAt + }, { + pack: (_.omit(op, "expiresAt") for op in @ops[512...1024]) + v: 512 + v_end: 1023 + meta: {start_ts: 512, end_ts: 1024} + n: 512 + sz: 56392 + expiresAt: @ops[1023].expiresAt + }] + done() + + it "should call the callback", -> + @callback.called.should.equal true + + describe "with temporary ops spanning more than 1 day", -> + beforeEach -> + @ops = ({ _id: "#{n}", op: "op-#{n}", meta: {start_ts: n*3600*1000, end_ts: n*3600*1000+1, user_id: "u#{n}"}, v: n, expiresAt: n+24*3600*1000 } for n in [0...48]) + @PackManager.convertDocsToPacks @ops, @callback + + it "should create two packs", (done) -> + @PackManager.convertDocsToPacks @ops, (err, packs) => + assert.deepEqual packs, [ { + pack: (_.omit(op, "expiresAt") for op in @ops[0...24]) + v: 0 + v_end: 23 + meta: {start_ts: 0 , end_ts: 23*3600*1000+1} + n: 24 + sz: 2538 + expiresAt: @ops[23].expiresAt + }, { + pack: (_.omit(op, "expiresAt") for op in @ops[24...48]) + v: 24 + v_end: 47 + meta: {start_ts: 24*3600*1000, end_ts: 47*3600*1000+1} + n: 24 + sz: 2568 + expiresAt: @ops[47].expiresAt + }] + done() + + it "should call the callback", -> + @callback.called.should.equal true + + describe "with mixture of temporary and permanent ops", -> + beforeEach -> + @ops = ({ _id: "#{n}", op: "op-#{n}", meta: {start_ts: n*3600*1000, end_ts: n*3600*1000+1, user_id: "u#{n}"}, v: n, expiresAt: n+24*3600*1000 } for n in [0...48]) + for n in [10...48] + delete @ops[n].expiresAt + @PackManager.convertDocsToPacks @ops, @callback + + it "should create two packs", (done) -> + @PackManager.convertDocsToPacks @ops, (err, packs) => + assert.deepEqual packs, [ { + pack: (_.omit(op, "expiresAt") for op in @ops[0...10]) + v: 0 + v_end: 9 + meta: {start_ts: 0 , end_ts: 9*3600*1000+1} + n: 10 + sz: 1040 + expiresAt: @ops[9].expiresAt + }, { + pack: (_.omit(op, "expiresAt") for op in @ops[10...48]) + v: 10 + v_end: 47 + meta: {start_ts: 10*3600*1000, end_ts: 47*3600*1000+1} + n: 38 + sz: 3496 + }] + done() + + it "should call the callback", -> + @callback.called.should.equal true + + describe "with mixture of temporary and permanent ops and an existing pack", -> + beforeEach -> + @ops = ({ _id: "#{n}", op: "op-#{n}", meta: {start_ts: n*3600*1000, end_ts: n*3600*1000+1, user_id: "u#{n}"}, v: n, expiresAt: n+24*3600*1000 } for n in [0...48]) + for n in [10...48] + delete @ops[n].expiresAt + # convert op 16 into a pack + @ops[16].pack = [ @ops[16].op ] + delete @ops[16].op + @PackManager.convertDocsToPacks @ops, @callback + + it "should create three packs", (done) -> + @PackManager.convertDocsToPacks @ops, (err, packs) => + assert.deepEqual packs, [ { + pack: (_.omit(op, "expiresAt") for op in @ops[0...10]) + v: 0 + v_end: 9 + meta: {start_ts: 0 , end_ts: 9*3600*1000+1} + n: 10 + sz: 1040 + expiresAt: @ops[9].expiresAt + }, { + pack: @ops[10...16] + v: 10 + v_end: 15 + meta: {start_ts: 10*3600*1000, end_ts: 15*3600*1000+1} + n: 6 + sz: 552 + }, { + pack: @ops[17...48] + v: 17 + v_end: 47 + meta: {start_ts: 17*3600*1000, end_ts: 47*3600*1000+1} + n: 31 + sz: 2852 + }] + done() + + it "should call the callback", -> + @callback.called.should.equal true