From 4efbd1e19ccabb7289aab11dcc01f1f5b96fc740 Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Tue, 8 Aug 2023 15:06:20 -0300 Subject: [PATCH 01/11] Support cas callback with prev being null --- index.js | 2 ++ test/cas.js | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/index.js b/index.js index 96790d01..082d5559 100644 --- a/index.js +++ b/index.js @@ -774,6 +774,8 @@ class Batch { node = await node.getChildNode(i) } + if (!node.block && cas && !(await cas(null, newNode))) return this._unlockMaybe() + let needsSplit = !(await node.insertKey(target, null, newNode, encoding, cas)) if (!node.changed) return this._unlockMaybe() diff --git a/test/cas.js b/test/cas.js index 777f9cb0..04c34289 100644 --- a/test/cas.js +++ b/test/cas.js @@ -2,6 +2,64 @@ const test = require('brittle') const b4a = require('b4a') const { create } = require('./helpers') +test('cas is called when prev does not exists', async function (t) { + t.plan(5) + + const db = create() + + t.comment('first put') + + await db.put('/a', '1', { + cas: function (prev, next) { + t.comment('first cb') + + t.is(prev, null) + t.alike(next, { seq: 1, key: '/a', value: '1' }) + + return true + } + }) + + t.comment('second put') + + await db.put('/a', '1', { + cas: function (prev, next) { + t.comment('second cb') + + t.alike(prev, { seq: 1, key: '/a', value: '1' }) + t.alike(next, { seq: 2, key: '/a', value: '1' }) + + return true + } + }) + + t.alike(await db.get('/a'), { seq: 2, key: '/a', value: '1' }) +}) + +test('cas is respected when prev does not exists', async function (t) { + t.plan(5) + + const db = create() + + await db.put('/a', '1', { + cas: function (prev, next) { + t.is(prev, null) + t.alike(next, { seq: 1, key: '/a', value: '1' }) + return false + } + }) + + await db.put('/a', '1', { + cas: function (prev, next) { + t.is(prev, null) + t.alike(next, { seq: 1, key: '/a', value: '1' }) + return false + } + }) + + t.is(await db.get('/a'), null) +}) + test('bee.put({ cas }) succeds if cas(last, next) returns truthy', async function (t) { const key = 'key' const value = 'value' From e2d8383e663bbe3f011e545a2af99bb3d9226953 Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Tue, 8 Aug 2023 16:44:49 -0300 Subject: [PATCH 02/11] Fix cas special case --- index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 082d5559..afa075d6 100644 --- a/index.js +++ b/index.js @@ -104,6 +104,10 @@ class TreeNode { let e = this.keys.length let c + if (e === 0) { + if (cas && !(await cas(null, node))) return true + } + while (s < e) { const mid = (s + e) >> 1 c = b4a.compare(key.value, await this.getKey(mid)) @@ -774,8 +778,6 @@ class Batch { node = await node.getChildNode(i) } - if (!node.block && cas && !(await cas(null, newNode))) return this._unlockMaybe() - let needsSplit = !(await node.insertKey(target, null, newNode, encoding, cas)) if (!node.changed) return this._unlockMaybe() From c97529befb1a27eda8d1256413df12e6219b2c63 Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Tue, 8 Aug 2023 16:45:04 -0300 Subject: [PATCH 03/11] Improve tests so it's clearer --- test/cas.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/test/cas.js b/test/cas.js index 04c34289..f8d152c8 100644 --- a/test/cas.js +++ b/test/cas.js @@ -3,7 +3,7 @@ const b4a = require('b4a') const { create } = require('./helpers') test('cas is called when prev does not exists', async function (t) { - t.plan(5) + t.plan(6) const db = create() @@ -20,24 +20,26 @@ test('cas is called when prev does not exists', async function (t) { } }) + t.alike(await db.get('/a'), { seq: 1, key: '/a', value: '1' }) + t.comment('second put') - await db.put('/a', '1', { + await db.put('/a', '2', { cas: function (prev, next) { t.comment('second cb') t.alike(prev, { seq: 1, key: '/a', value: '1' }) - t.alike(next, { seq: 2, key: '/a', value: '1' }) + t.alike(next, { seq: 2, key: '/a', value: '2' }) return true } }) - t.alike(await db.get('/a'), { seq: 2, key: '/a', value: '1' }) + t.alike(await db.get('/a'), { seq: 2, key: '/a', value: '2' }) }) -test('cas is respected when prev does not exists', async function (t) { - t.plan(5) +test.solo('cas is respected when prev does not exists', async function (t) { + t.plan(6) const db = create() @@ -49,10 +51,12 @@ test('cas is respected when prev does not exists', async function (t) { } }) - await db.put('/a', '1', { + t.is(await db.get('/a'), null) + + await db.put('/a', '2', { cas: function (prev, next) { t.is(prev, null) - t.alike(next, { seq: 1, key: '/a', value: '1' }) + t.alike(next, { seq: 1, key: '/a', value: '2' }) return false } }) From b394e593788a3b08e982d0650b0cd1ad062ea054 Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Tue, 8 Aug 2023 16:54:48 -0300 Subject: [PATCH 04/11] Fix bug seq increasing --- index.js | 5 ++++- test/cas.js | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index afa075d6..8824fc0b 100644 --- a/index.js +++ b/index.js @@ -105,7 +105,10 @@ class TreeNode { let c if (e === 0) { - if (cas && !(await cas(null, node))) return true + if (cas && !(await cas(null, node))) { + this.changed = false + return true + } } while (s < e) { diff --git a/test/cas.js b/test/cas.js index f8d152c8..101f0f47 100644 --- a/test/cas.js +++ b/test/cas.js @@ -38,7 +38,7 @@ test('cas is called when prev does not exists', async function (t) { t.alike(await db.get('/a'), { seq: 2, key: '/a', value: '2' }) }) -test.solo('cas is respected when prev does not exists', async function (t) { +test('cas is respected when prev does not exists', async function (t) { t.plan(6) const db = create() From b7d67ffb00f6fb29d4b63c75cb560043c562bc2a Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Tue, 8 Aug 2023 17:05:46 -0300 Subject: [PATCH 05/11] Clearer --- index.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/index.js b/index.js index 8824fc0b..00878c27 100644 --- a/index.js +++ b/index.js @@ -104,13 +104,6 @@ class TreeNode { let e = this.keys.length let c - if (e === 0) { - if (cas && !(await cas(null, node))) { - this.changed = false - return true - } - } - while (s < e) { const mid = (s + e) >> 1 c = b4a.compare(key.value, await this.getKey(mid)) @@ -126,6 +119,13 @@ class TreeNode { else s = mid + 1 } + if (this.keys.length === 0) { + if (cas && !(await cas(null, node))) { + this.changed = false + return true + } + } + const i = c < 0 ? e : s this.keys.splice(i, 0, key) if (child) this.children.splice(i + 1, 0, new Child(0, 0, child)) From 12410133b9feafa27533630ac936fd84fc87586a Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Tue, 8 Aug 2023 17:20:22 -0300 Subject: [PATCH 06/11] Remove special case if --- index.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 00878c27..9079b680 100644 --- a/index.js +++ b/index.js @@ -119,11 +119,9 @@ class TreeNode { else s = mid + 1 } - if (this.keys.length === 0) { - if (cas && !(await cas(null, node))) { - this.changed = false - return true - } + if (cas && !(await cas(null, node))) { + this.changed = false + return true } const i = c < 0 ? e : s From 0811b653bd20f08f98b6412e587f8b2360d6e55c Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Mon, 14 Aug 2023 14:12:11 -0300 Subject: [PATCH 07/11] Allow modifying next.value --- index.js | 5 ++--- test/cas.js | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index 9079b680..14340262 100644 --- a/index.js +++ b/index.js @@ -741,7 +741,6 @@ class Batch { value } key = enc(encoding.key, key) - value = enc(encoding.value, value) const stack = [] @@ -768,7 +767,7 @@ class Batch { if (cas && !(await cas((await node.getKeyNode(mid)).final(encoding), newNode))) return this._unlockMaybe() node.setKey(mid, target) - return this._append(root, seq, key, value) + return this._append(root, seq, key, enc(encoding.value, newNode.value)) } if (c < 0) e = mid @@ -798,7 +797,7 @@ class Batch { } } - return this._append(root, seq, key, value) + return this._append(root, seq, key, enc(encoding.value, newNode.value)) } async del (key, opts) { diff --git a/test/cas.js b/test/cas.js index 101f0f47..72bdcfc9 100644 --- a/test/cas.js +++ b/test/cas.js @@ -2,6 +2,62 @@ const test = require('brittle') const b4a = require('b4a') const { create } = require('./helpers') +test('cas - swap with a new value', async function (t) { + const db = create({ valueEncoding: 'json' }) + + await db.put('/a', 0, { + cas: function (prev, next) { + t.is(prev, null) + t.alike(next, { seq: 1, key: '/a', value: 0 }) + return next + } + }) + + t.alike(await db.get('/a'), { seq: 1, key: '/a', value: 0 }) + + await db.put('/a', 99, { + cas: function (prev, next) { + t.alike(prev, { seq: 1, key: '/a', value: 0 }) + t.alike(next, { seq: 2, key: '/a', value: 99 }) + next.value = prev.value + 1 // Overwrites so it's not 99 anymore + return next + } + }) + + t.alike(await db.get('/a'), { seq: 2, key: '/a', value: 1 }) + + await db.put('/a', 99, { + cas: function (prev, next) { + t.alike(prev, { seq: 2, key: '/a', value: 1 }) + t.alike(next, { seq: 3, key: '/a', value: 99 }) + next.value = prev.value + 1 + return next + } + }) + + t.alike(await db.get('/a'), { seq: 3, key: '/a', value: 2 }) +}) + +test('cas - should not swap', async function (t) { + t.plan(4) + + const db = create({ valueEncoding: 'json' }) + + await db.put('/a', 1) + + t.alike(await db.get('/a'), { seq: 1, key: '/a', value: 1 }) + + await db.put('/a', 2, { + cas: function (prev, next) { + t.alike(prev, { seq: 1, key: '/a', value: 1 }) + t.alike(next, { seq: 2, key: '/a', value: 2 }) + return null + } + }) + + t.alike(await db.get('/a'), { seq: 1, key: '/a', value: 1 }) +}) + test('cas is called when prev does not exists', async function (t) { t.plan(6) From 2bcb3d3abbc2f414362173520a1cf8062e738b74 Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Mon, 14 Aug 2023 14:42:18 -0300 Subject: [PATCH 08/11] Fix swap to keep older one --- index.js | 9 +++++++-- test/cas.js | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 14340262..0b2d072a 100644 --- a/index.js +++ b/index.js @@ -109,7 +109,10 @@ class TreeNode { c = b4a.compare(key.value, await this.getKey(mid)) if (c === 0) { - if (cas && !(await cas((await this.getKeyNode(mid)).final(encoding), node))) return true + const older = (await this.getKeyNode(mid)).final(encoding) + const swap = cas ? (await cas(older, node)) : null + if (cas && (!swap || swap === older)) return true + this.changed = true this.keys[mid] = key return true @@ -764,7 +767,9 @@ class Batch { c = b4a.compare(target.value, await node.getKey(mid)) if (c === 0) { - if (cas && !(await cas((await node.getKeyNode(mid)).final(encoding), newNode))) return this._unlockMaybe() + const older = (await node.getKeyNode(mid)).final(encoding) + const swap = cas ? (await cas(older, newNode)) : null + if (cas && (!swap || swap === older)) return this._unlockMaybe() node.setKey(mid, target) return this._append(root, seq, key, enc(encoding.value, newNode.value)) diff --git a/test/cas.js b/test/cas.js index 72bdcfc9..992a18f7 100644 --- a/test/cas.js +++ b/test/cas.js @@ -58,6 +58,24 @@ test('cas - should not swap', async function (t) { t.alike(await db.get('/a'), { seq: 1, key: '/a', value: 1 }) }) +test('cas - swap but keep older one', async function (t) { + const db = create({ valueEncoding: 'json' }) + + await db.put('/a', 0) + + t.alike(await db.get('/a'), { seq: 1, key: '/a', value: 0 }) + + await db.put('/a', 99, { + cas: function (prev, next) { + t.alike(prev, { seq: 1, key: '/a', value: 0 }) + t.alike(next, { seq: 2, key: '/a', value: 99 }) + return prev + } + }) + + t.alike(await db.get('/a'), { seq: 1, key: '/a', value: 0 }) +}) + test('cas is called when prev does not exists', async function (t) { t.plan(6) From 71fc3f87256945316d928be4e917b7fc4632dafc Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Mon, 14 Aug 2023 15:17:56 -0300 Subject: [PATCH 09/11] Fix swap at del --- index.js | 5 ++++- test/cas.js | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index 0b2d072a..7aa7cb0f 100644 --- a/index.js +++ b/index.js @@ -848,7 +848,10 @@ class Batch { c = b4a.compare(key, await node.getKey(mid)) if (c === 0) { - if (cas && !(await cas((await node.getKeyNode(mid)).final(encoding), delNode))) return this._unlockMaybe() + const older = (await node.getKeyNode(mid)).final(encoding) + const swap = cas ? (await cas(older, delNode)) : null + if (cas && (!swap || swap === older)) return this._unlockMaybe() + if (node.children.length) await setKeyToNearestLeaf(node, mid, stack) else node.removeKey(mid) // we mark these as changed late, so we don't rewrite them if it is a 404 diff --git a/test/cas.js b/test/cas.js index 992a18f7..cde0e896 100644 --- a/test/cas.js +++ b/test/cas.js @@ -76,6 +76,32 @@ test('cas - swap but keep older one', async function (t) { t.alike(await db.get('/a'), { seq: 1, key: '/a', value: 0 }) }) +test('cas - swap deletion', async function (t) { + const db = create({ valueEncoding: 'json' }) + + await db.put('/a', 0) + + await db.del('/a', { + cas: function (prev, next) { + t.alike(prev, { seq: 1, key: '/a', value: 0 }) + t.alike(next, { seq: 2, key: '/a', value: null }) + return prev + } + }) + + t.alike(await db.get('/a'), { seq: 1, key: '/a', value: 0 }) + + await db.del('/a', { + cas: function (prev, next) { + t.alike(prev, { seq: 1, key: '/a', value: 0 }) + t.alike(next, { seq: 2, key: '/a', value: null }) + return next + } + }) + + t.alike(await db.get('/a'), null) +}) + test('cas is called when prev does not exists', async function (t) { t.plan(6) From 4b8bd6659ed5c9202eaebcaa4d3bfa5a567e2ff3 Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Mon, 14 Aug 2023 15:20:17 -0300 Subject: [PATCH 10/11] Update README --- README.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index dc9af1a0..2d827080 100644 --- a/README.md +++ b/README.md @@ -116,14 +116,16 @@ If you're inserting a series of data atomically or want more performance then ch `options` includes: ```js { - cas (prev, next) { return true } + cas (prev, next) { return next } } ``` ##### Compare And Swap (cas) `cas` option is a function comparator to control whether the `put` succeeds. -By returning `true` it will insert the value, otherwise it won't. +By returning `next` it will insert the value, otherwise it won't. + +It's allowed to change `next.value` only. It receives two args: `prev` is the current node entry, and `next` is the potential new node. @@ -140,7 +142,7 @@ console.log(await db.get('number')) // => { seq: 2, key: 'number', value: '456' function cas (prev, next) { // You can use same-data or same-object lib, depending on the value complexity - return prev.value !== next.value + return prev.value !== next.value ? next : prev } ``` @@ -157,7 +159,7 @@ Delete a key. `options` include: ```js { - cas (prev, next) { return true } + cas (prev, next) { return next } } ``` @@ -180,7 +182,7 @@ await db.del('number', { cas }) console.log(await db.get('number')) // => null function cas (prev) { - return prev.value === 'can-be-deleted' + return prev.value === 'can-be-deleted' ? next : prev } ``` From 0fdfc4ef99e4e227e3708eb8f3ff9d0ddfe58674 Mon Sep 17 00:00:00 2001 From: Lucas Barrena Date: Sat, 23 Sep 2023 13:04:26 -0300 Subject: [PATCH 11/11] Feedback --- index.js | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/index.js b/index.js index 7aa7cb0f..368720d6 100644 --- a/index.js +++ b/index.js @@ -109,9 +109,11 @@ class TreeNode { c = b4a.compare(key.value, await this.getKey(mid)) if (c === 0) { - const older = (await this.getKeyNode(mid)).final(encoding) - const swap = cas ? (await cas(older, node)) : null - if (cas && (!swap || swap === older)) return true + if (cas) { + const older = (await this.getKeyNode(mid)).final(encoding) + const swap = await cas(older, node) + if (!swap || swap === older) return true + } this.changed = true this.keys[mid] = key @@ -767,12 +769,16 @@ class Batch { c = b4a.compare(target.value, await node.getKey(mid)) if (c === 0) { - const older = (await node.getKeyNode(mid)).final(encoding) - const swap = cas ? (await cas(older, newNode)) : null - if (cas && (!swap || swap === older)) return this._unlockMaybe() + if (cas) { + const older = (await node.getKeyNode(mid)).final(encoding) + const swap = await cas(older, newNode) + if (!swap || swap === older) return this._unlockMaybe() + } node.setKey(mid, target) - return this._append(root, seq, key, enc(encoding.value, newNode.value)) + + const valueEncoded = enc(encoding.value, newNode.value) + return this._append(root, seq, key, valueEncoded) } if (c < 0) e = mid @@ -802,7 +808,8 @@ class Batch { } } - return this._append(root, seq, key, enc(encoding.value, newNode.value)) + const valueEncoded = enc(encoding.value, newNode.value) + return this._append(root, seq, key, valueEncoded) } async del (key, opts) { @@ -848,9 +855,11 @@ class Batch { c = b4a.compare(key, await node.getKey(mid)) if (c === 0) { - const older = (await node.getKeyNode(mid)).final(encoding) - const swap = cas ? (await cas(older, delNode)) : null - if (cas && (!swap || swap === older)) return this._unlockMaybe() + if (cas) { + const older = (await node.getKeyNode(mid)).final(encoding) + const swap = await cas(older, delNode) + if (!swap || swap === older) return this._unlockMaybe() + } if (node.children.length) await setKeyToNearestLeaf(node, mid, stack) else node.removeKey(mid)