From a3f143051be7262dfa5175deb85b5ae305d00e0d Mon Sep 17 00:00:00 2001 From: Youngteac Hong Date: Tue, 10 Dec 2024 19:45:08 +0900 Subject: [PATCH] Revise the codes --- pkg/document/change/id.go | 14 ++++++++++---- pkg/document/internal_document.go | 8 +++++++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/pkg/document/change/id.go b/pkg/document/change/id.go index 13c36071f..45b0607eb 100644 --- a/pkg/document/change/id.go +++ b/pkg/document/change/id.go @@ -104,8 +104,9 @@ func (id ID) SyncClocks(other ID) ID { lamport = other.lamport + 1 } - // NOTE(chacha912): Handle changes from legacy SDK (v0.5.2 or below) which have empty version vectors. - // Add lamport to version vector to include this change's information. + // NOTE(chacha912): For changes created by legacy SDK prior to v0.5.2 that lack version + // vectors, document's version vector was not being properly accumlated. To address this, + // we generate a version vector using the lamport timestamp when no version vector exists. otherVV := other.versionVector if len(otherVV) == 0 { otherVV = otherVV.DeepCopy() @@ -125,8 +126,13 @@ func (id ID) SetClocks(otherLamport int64, vector time.VersionVector) ID { lamport = otherLamport + 1 } - // NOTE(chacha912): Snapshot's version vector from server contains initialActorID. - // Remove it as it's not an actual participating client. + // NOTE(chacha912): Documents created by server may have an InitialActorID + // in their version vector. Although server is not an actual client, it + // generates document snapshots from changes by participating with an + // InitialActorID during document instance creation and accumulating stored + // changes in DB. + // Semantically, including a non-client actor in version vector is + // problematic. To address this, we remove the InitialActorID from snapshots. vector.Unset(time.InitialActorID) newID := NewID(id.clientSeq, id.serverSeq, lamport, id.actorID, id.versionVector.Max(vector)) diff --git a/pkg/document/internal_document.go b/pkg/document/internal_document.go index 916d79ab5..c693d82a9 100644 --- a/pkg/document/internal_document.go +++ b/pkg/document/internal_document.go @@ -269,7 +269,13 @@ func (d *InternalDocument) applySnapshot(snapshot []byte, vector time.VersionVec d.root = crdt.NewRoot(rootObj) d.presences = presences - // TODO(chacha912): We need to check we can use max lamport as lamport timestamp. + // NOTE(chacha912): Documents created from snapshots were experiencing edit + // restrictions due to low lamport values. + // Previously, the code attempted to generate document lamport from ServerSeq. + // However, after aligning lamport logic with the original research paper, + // ServerSeq could potentially become smaller than the lamport value. + // To resolve this, we initialize document's lamport by using the highest + // lamport value stored in version vector as the starting point. d.changeID = d.changeID.SetClocks(vector.MaxLamport(), vector) return nil