From b7e73a938e3ded1438c219348259161727aa8705 Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Mon, 30 Mar 2026 00:28:26 +1100 Subject: [PATCH] fix rapid barrier use --- server/pkg/server/server.go | 2 +- server/pkg/ylayer/doc.go | 23 +++++++--------------- server/pkg/ylayer/node.go | 35 ++++++++++++++++++++++++++++------ server/pkg/ylayer/types.go | 4 ++-- tests/integration/data.test.ts | 4 ++-- 5 files changed, 41 insertions(+), 27 deletions(-) diff --git a/server/pkg/server/server.go b/server/pkg/server/server.go index a94d83f..81177f6 100644 --- a/server/pkg/server/server.go +++ b/server/pkg/server/server.go @@ -32,7 +32,7 @@ func (sd *safeDoc) Perform(in InPacket, handleID, clientID int) (ok bool, err er sd.lock.Lock() defer sd.lock.Unlock() - if !sd.doc.Barrier(in.TargetVersion, in.Barrier) { + if !sd.doc.Barrier(handleID, in.TargetVersion, in.Barrier) { // not an error - failed to pass barrier // TODO: we don't explicitly announce this - expect client to know return diff --git a/server/pkg/ylayer/doc.go b/server/pkg/ylayer/doc.go index 5f1fc30..6694e5c 100644 --- a/server/pkg/ylayer/doc.go +++ b/server/pkg/ylayer/doc.go @@ -147,23 +147,14 @@ func (d *docImpl) Read() (dw *DocWork) { return } -// inoToVersion returns the high version for the specific inode. -// This is for the caller to check whether work can be applied (check barrier). -func (d *docImpl) inoToVersion(ino string) (version int) { - nh := d.byIno[ino] - if nh == nil { - return 0 - } - _, version = nh.versionRange() - return -} - -func (d *docImpl) Barrier(targetVersion int, inos []string) (ok bool) { - if targetVersion <= 0 { - return true - } +func (d *docImpl) Barrier(handle, targetVersion int, inos []string) (ok bool) { for _, ino := range inos { - if d.inoToVersion(ino) > targetVersion { + nh := d.byIno[ino] + if nh == nil { + continue + } + + if !nh.allowBarrier(handle, targetVersion) { return false } } diff --git a/server/pkg/ylayer/node.go b/server/pkg/ylayer/node.go index 068182f..c11d339 100644 --- a/server/pkg/ylayer/node.go +++ b/server/pkg/ylayer/node.go @@ -30,15 +30,17 @@ type nodeHolder struct { dataAt int // when was data last flattened data GumnutDataArray // data at dataAt + prune bool // if history is not exhaustive history []historyEntry // history over time length int // current length } -func (fn *nodeHolder) versionRange() (lo, hi int) { - if len(fn.history) == 0 { - return 0, 0 +func (fn *nodeHolder) versionRange() (prune bool, lo, hi int) { + if len(fn.history) != 0 { + lo, hi = fn.history[0].version, fn.history[len(fn.history)-1].version } - return fn.history[0].version, fn.history[len(fn.history)-1].version + prune = fn.prune + return } // read moves the current "live" version here forward to the given version number. @@ -97,6 +99,27 @@ func (nh *nodeHolder) clearHandle(handle int) (change bool) { return } +// allowBarrier checks that the handle with the given target is "as they see it". +// i.e., there are only ops past the target which the handle created. +func (n *nodeHolder) allowBarrier(handle, targetVersion int) (allow bool) { + prune, lo, _ := n.versionRange() + if prune && targetVersion < lo { + return false + } + + // look for the first entry that is > the known version + idx := sort.Search(len(n.history), func(i int) (match bool) { + return n.history[i].version > targetVersion + }) + + for _, entry := range n.history[idx:] { + if handle != entry.handle { + return false + } + } + return true +} + func (nh *nodeHolder) historyFrom(version, handle int) (out []ymodel.PriorOpSelf) { // look for the first entry that is > the known version idx := sort.Search(len(nh.history), func(i int) (match bool) { @@ -128,8 +151,8 @@ type prepareResult struct { // prepare generates the updated state we should apply to this nodeHolder after this Patch. // This is so doc can apply ops on many nodes at once. func (nh *nodeHolder) prepare(arg prepareArg, p *Patch) (res prepareResult, err error) { - lo, hi := nh.versionRange() - if arg.targetVersion > 0 && arg.targetVersion < lo { + prune, lo, hi := nh.versionRange() + if prune && arg.targetVersion > 0 && arg.targetVersion < lo { err = ErrVersion return // must be within known range - but we don't check hi, assume host has } diff --git a/server/pkg/ylayer/types.go b/server/pkg/ylayer/types.go index 4383992..a8496ac 100644 --- a/server/pkg/ylayer/types.go +++ b/server/pkg/ylayer/types.go @@ -18,9 +18,9 @@ type Doc interface { // Internally, this may apply ops forward: we don't always keep a "live" copy around. Read() (dw *DocWork) - // Barrier returns true if the given client can pass the barrier with the given inos and their known target version. + // Barrier returns true if the given handle can pass the barrier with the given inos and their known target version. // This is effectively checking all the target inos have not changed since ... - Barrier(targetVersion int, inos []string) (ok bool) + Barrier(handle, targetVersion int, inos []string) (ok bool) // ClearHandle removes the given handle from known history. ClearHandle(handleID int) (change bool) diff --git a/tests/integration/data.test.ts b/tests/integration/data.test.ts index 5e5e2d5..16f7043 100644 --- a/tests/integration/data.test.ts +++ b/tests/integration/data.test.ts @@ -13,12 +13,12 @@ test('data', async (t) => { data1.root().set('abc', 'hello'); data1.root().set('abc', 'hello123'); - // data1.root().set('def', data1.root()); // FIXME: this is failing to apply over itself - seq3 + data1.root().set('def', data1.root()); await checkCond(() => { const root = data2.root(); assert.strictEqual(root.get('abc'), 'hello123'); - // assert.strictEqual(root.get('def'), root); + assert.strictEqual(root.get('def'), root); }); });