From 88971b4e69b7be080147e0b4f17d6bfe51904a3f Mon Sep 17 00:00:00 2001 From: Kevin Jahns <kevin.jahns@rwth-aachen.de> Date: Mon, 21 Mar 2016 21:00:28 +0100 Subject: [PATCH] fixed several issues of the gc. I.e. the gc sometimes did not collect the whole subtree when deleting an operation --- src/Database.js | 40 ++++++++++++----- src/SpecHelper.js | 4 +- src/Transaction.js | 110 ++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 129 insertions(+), 25 deletions(-) diff --git a/src/Database.js b/src/Database.js index 5b2daf50..0b55cfec 100644 --- a/src/Database.js +++ b/src/Database.js @@ -110,6 +110,18 @@ module.exports = function (Y /* :any */) { garbageCollect() } } + emptyGarbageCollector () { + return new Promise (resolve => { + var check = () => { + if (this.gc1.length > 0 || this.gc2.length > 0) { + this.garbageCollect().then(check) + } else { + resolve() + } + } + setTimeout(check, 0) + }) + } addToDebug () { if (typeof YConcurrency_TestingMode !== 'undefined') { var command /* :string */ = Array.prototype.map.call(arguments, function (s) { @@ -339,15 +351,7 @@ module.exports = function (Y /* :any */) { } } else { // increase SS - var o = op - var state = yield* transaction.getState(op.id[0]) - while (o != null && o.id[1] === state.clock && op.id[0] === o.id[0]) { - // either its a new operation (1. case), or it is an operation that was deleted, but is not yet in the OS - state.clock++ - yield* transaction.checkDeleteStoreForState(state) - o = yield* transaction.os.findNext(o.id) - } - yield* transaction.setState(state) + yield* transaction.updateState(op.id[0]) // notify whenOperation listeners (by id) var sid = JSON.stringify(op.id) @@ -364,6 +368,15 @@ module.exports = function (Y /* :any */) { } var t = this.initializedTypes[JSON.stringify(op.parent)] + // if parent is deleted, mark as gc'd and return + if (op.parent != null) { + var parentIsDeleted = yield* transaction.isDeleted(op.parent) + if (parentIsDeleted) { + yield* transaction.deleteList(op.id) + return + } + } + // Delete if DS says this is actually deleted var opIsDeleted = yield* transaction.isDeleted(op.id) if (!op.deleted && opIsDeleted) { @@ -375,8 +388,13 @@ module.exports = function (Y /* :any */) { } // notify parent, if it was instanciated as a custom type - if (t != null && !opIsDeleted) { - yield* t._changed(transaction, Y.utils.copyObject(op)) + if (t != null) { + let o = Y.utils.copyObject(op) + if (opIsDeleted && !o.deleted) { + // op did not reflect the created delete op (happens when not using y-memory) + o.deleted = true + } + yield* t._changed(transaction, o) } } } diff --git a/src/SpecHelper.js b/src/SpecHelper.js index d5daca9d..72435424 100644 --- a/src/SpecHelper.js +++ b/src/SpecHelper.js @@ -139,9 +139,9 @@ g.applyRandomTransactionsWithGC = async(function * applyRandomTransactions (user g.garbageCollectAllUsers = async(function * garbageCollectAllUsers (users) { // gc two times because of the two gc phases (really collect everything) + yield wait(100) for (var i in users) { - yield users[i].db.garbageCollect() - yield users[i].db.garbageCollect() + yield users[i].db.emptyGarbageCollector() } }) diff --git a/src/Transaction.js b/src/Transaction.js index ee1556cf..b8ef6a46 100644 --- a/src/Transaction.js +++ b/src/Transaction.js @@ -154,17 +154,32 @@ module.exports = function (Y/* :any */) { } * deleteList (start) { - if (this.store.y.connector.isSynced) { - while (start != null && this.store.y.connector.isSynced) { - start = yield* this.getOperation(start) + while (start != null) { + start = yield* this.getOperation(start) + if (start.gc) { + break + } else { start.gc = true + start.deleted = true yield* this.setOperation(start) - // TODO: will always reset the parent.. - this.store.gc1.push(start.id) + yield* this.markDeleted(start.id, 1) + if (start.opContent != null) { + yield* this.deleteOperation(start.opContent) + /* + yield* this.deleteOperation(start.opContent) + var opContent = yield* this.getOperation(start.opContent) + opContent.gc = true + yield* this.setOperation(opContent) + if (this.store.y.connector.isSynced) { + this.store.gc1.push(opContent.id) + } + */ + } + if (this.store.y.connector.isSynced){ + this.store.gc1.push(start.id) + } start = start.right } - } else { - // TODO: when not possible??? do later in (gcWhenSynced) } } @@ -199,19 +214,24 @@ module.exports = function (Y/* :any */) { if (target.start != null) { // TODO: don't do it like this .. -.- yield* this.deleteList(target.start) - yield* this.deleteList(target.id) + // yield* this.deleteList(target.id) -- do not gc itself because this may still get referenced } if (target.map != null) { for (var name in target.map) { yield* this.deleteList(target.map[name]) } // TODO: here to.. (see above) - yield* this.deleteList(target.id) + // yield* this.deleteList(target.id) -- see above } if (target.opContent != null) { yield* this.deleteOperation(target.opContent) // target.opContent = null } + if (target.requires != null) { + for (var i = 0; i < target.requires.length; i++) { + yield* this.deleteOperation(target.requires[i]) + } + } } var left if (target.left != null) { @@ -377,9 +397,40 @@ module.exports = function (Y/* :any */) { */ * garbageCollectAfterSync () { yield* this.os.iterate(this, null, null, function * (op) { - if (op.deleted && op.left != null) { - var left = yield* this.getOperation(op.left) - this.store.addToGarbageCollector(op, left) + if (op.gc) { + this.store.gc1.push(op.id) + } else { + if (op.parent != null) { + var parentDeleted = yield* this.isDeleted(op.parent) + if (parentDeleted) { + op.gc = true + if (!op.deleted) { + yield* this.markDeleted(op.id, 1) + op.deleted = true + if (op.opContent != null) { + yield* this.deleteOperation(op.opContent) + /* + var opContent = yield* this.getOperation(op.opContent) + opContent.gc = true + yield* this.setOperation(opContent) + this.store.gc1.push(opContent.id) + */ + } + if (op.requires != null) { + for (var i = 0; i < op.requires.length; i++) { + yield* this.deleteOperation(op.requires[i]) + } + } + } + yield* this.setOperation(op) + this.store.gc1.push(op.id) + return + } + } + if (op.deleted && op.left != null) { + var left = yield* this.getOperation(op.left) + this.store.addToGarbageCollector(op, left) + } } }) } @@ -404,6 +455,29 @@ module.exports = function (Y/* :any */) { o = yield* this.getOperation(id) } */ + + var deps = [] + if (o.opContent != null) { + deps.push(o.opContent) + } + if (o.requires != null) { + deps = deps.concat(o.requires) + } + for (var i = 0; i < deps.length; i++) { + var dep = yield* this.getOperation(deps[i]) + if (dep != null) { + if (!dep.deleted) { + yield* this.deleteOperation(dep.id) + dep = yield* this.getOperation(dep.id) + } + dep.gc = true + yield* this.setOperation(dep) + this.store.gc1.push(dep.id) + } else { + yield* this.markGarbageCollected(deps[i], 1) + yield* this.updateState(deps[i][0]) // TODO: unneccessary? + } + } // remove gc'd op from the left op, if it exists if (o.left != null) { @@ -535,6 +609,18 @@ module.exports = function (Y/* :any */) { state.clock = Math.max(state.clock, n.id[1] + n.len) } } + * updateState (user) { + var state = yield* this.getState(user) + yield* this.checkDeleteStoreForState(state) + var o = yield* this.getOperation([user, state.clock]) + while (o != null && o.id[1] === state.clock && user === o.id[0]) { + // either its a new operation (1. case), or it is an operation that was deleted, but is not yet in the OS + state.clock++ + yield* this.checkDeleteStoreForState(state) + o = yield* this.os.findNext(o.id) + } + yield* this.setState(state) + } /* apply a delete set in order to get the state of the supplied ds -- GitLab