Skip to content

Commit

Permalink
Improve Version Vector Handling for Legacy SDK and Snapshots (#933)
Browse files Browse the repository at this point in the history 
This commit addressed critical version vector management issues across
multiple scenarios in document creation and editing. It implemented fixes for
legacy SDK changes, server-generated snapshots, and lamport timestamp
initialization to ensure accurate concurrent editing and version tracking.

---------

Co-authored-by: Youngteac Hong <susukang98@gmail.com>
  • Loading branch information
@chacha912 @hackerwins
chacha912 and hackerwins authored Dec 10, 2024
1 parent db02eb9 commit ba70776
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 6 deletions.
23 changes: 21 additions & 2 deletions packages/sdk/src/document/change/change_id.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,17 @@ export class ChangeID {
public syncClocks(other: ChangeID): ChangeID {
const lamport =
other.lamport > this.lamport ? other.lamport + 1n : this.lamport + 1n;
const maxVersionVector = this.versionVector.max(other.versionVector);

// 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.
let otherVV = other.versionVector;
if (otherVV.size() === 0) {
otherVV = otherVV.deepcopy();
otherVV.set(other.actor, other.lamport);
}

const maxVersionVector = this.versionVector.max(otherVV);
const newID = new ChangeID(
this.clientSeq,
lamport,
Expand All @@ -103,7 +112,17 @@ export class ChangeID {
*/
public setClocks(otherLamport: bigint, vector: VersionVector): ChangeID {
const lamport =
otherLamport > this.lamport ? otherLamport : this.lamport + 1n;
otherLamport > this.lamport ? otherLamport + 1n : this.lamport + 1n;

// 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(InitialActorID);

const maxVersionVector = this.versionVector.max(vector);
maxVersionVector.set(this.actor, lamport);

Expand Down
5 changes: 4 additions & 1 deletion packages/sdk/src/document/document.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1450,7 +1450,10 @@ export class Document<T, P extends Indexable = Indexable> {
const { root, presences } = converter.bytesToSnapshot<P>(snapshot);
this.root = new CRDTRoot(root);
this.presences = presences;
this.changeID = this.changeID.setClocks(serverSeq, snapshotVector);
this.changeID = this.changeID.setClocks(
snapshotVector.maxLamport(),
snapshotVector,
);

// drop clone because it is contaminated.
this.clone = undefined;
Expand Down
7 changes: 7 additions & 0 deletions packages/sdk/src/document/time/version_vector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ export class VersionVector {
this.vector.set(actorID, lamport);
}

/**
* `unset` removes the version for the given actor from the VersionVector.
*/
public unset(actorID: string): void {
this.vector.delete(actorID);
}

/**
* `get` gets the lamport timestamp of the given actor.
*/
Expand Down
135 changes: 132 additions & 3 deletions packages/sdk/test/integration/gc_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import {
testRPCAddr,
toDocKey,
} from '@yorkie-js-sdk/test/integration/integration_helper';
import { MaxVersionVector, versionVectorHelper } from '../helper/helper';
import {
MaxVersionVector,
versionVectorHelper,
DefaultSnapshotThreshold,
} from '../helper/helper';

describe('Garbage Collection', function () {
it('getGarbageLen should return the actual number of elements garbage-collected', async function ({
Expand Down Expand Up @@ -1701,7 +1705,7 @@ describe('Garbage Collection', function () {
task,
}) {
type TestDoc = { t: Text };
const docKey = toDocKey(`${task.name}-${new Date().getTime()}`);
const docKey = toDocKey(`${new Date().getTime()}-${task.name}`);
const doc1 = new yorkie.Document<TestDoc>(docKey);
const doc2 = new yorkie.Document<TestDoc>(docKey);
const client1 = new yorkie.Client(testRPCAddr);
Expand Down Expand Up @@ -1875,7 +1879,7 @@ describe('Garbage Collection', function () {
task,
}) {
type TestDoc = { t: Text };
const docKey = toDocKey(`${task.name}-${new Date().getTime()}`);
const docKey = toDocKey(`${new Date().getTime()}-${task.name}`);
const doc1 = new yorkie.Document<TestDoc>(docKey);
const doc2 = new yorkie.Document<TestDoc>(docKey);
const client1 = new yorkie.Client(testRPCAddr);
Expand Down Expand Up @@ -2226,4 +2230,129 @@ describe('Garbage Collection', function () {
await client2.deactivate();
await client3.deactivate();
});

it('snapshot version vector test', async function ({ task }) {
type TestDoc = { t: Text };
const docKey = toDocKey(`${task.name}-${new Date().getTime()}`);
const doc1 = new yorkie.Document<TestDoc>(docKey);
const doc2 = new yorkie.Document<TestDoc>(docKey);
const doc3 = new yorkie.Document<TestDoc>(docKey);
const client1 = new yorkie.Client(testRPCAddr);
const client2 = new yorkie.Client(testRPCAddr);
const client3 = new yorkie.Client(testRPCAddr);
await client1.activate();
await client2.activate();
await client3.activate();

await client1.attach(doc1, { syncMode: SyncMode.Manual });
await client2.attach(doc2, { syncMode: SyncMode.Manual });
await client3.attach(doc3, { syncMode: SyncMode.Manual });

doc1.update((root) => {
root.t = new Text();
root.t.edit(0, 0, 'a');
}, 'sets text');

await client1.sync();
await client2.sync();
await client3.sync();

assert.equal(
versionVectorHelper(doc1.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(4) },
{ actor: client2.getID()!, lamport: BigInt(1) },
{ actor: client3.getID()!, lamport: BigInt(1) },
]),
true,
);
assert.equal(
versionVectorHelper(doc2.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(2) },
{ actor: client2.getID()!, lamport: BigInt(4) },
{ actor: client3.getID()!, lamport: BigInt(1) },
]),
true,
);
assert.equal(
versionVectorHelper(doc3.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(2) },
{ actor: client2.getID()!, lamport: BigInt(1) },
{ actor: client3.getID()!, lamport: BigInt(4) },
]),
true,
);

// 01. Updates changes over snapshot threshold.
for (let idx = 0; idx < DefaultSnapshotThreshold / 2; idx++) {
doc1.update((root) => {
root.t.edit(0, 0, `${idx % 10}`);
});
await client1.sync();
await client2.sync();

doc2.update((root) => {
root.t.edit(0, 0, `${idx % 10}`);
});
await client2.sync();
await client1.sync();
}

assert.equal(
versionVectorHelper(doc1.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(2004) },
{ actor: client2.getID()!, lamport: BigInt(2003) },
{ actor: client3.getID()!, lamport: BigInt(1) },
]),
true,
);
assert.equal(
versionVectorHelper(doc2.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(2001) },
{ actor: client2.getID()!, lamport: BigInt(2003) },
{ actor: client3.getID()!, lamport: BigInt(1) },
]),
true,
);
assert.equal(
versionVectorHelper(doc3.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(2) },
{ actor: client2.getID()!, lamport: BigInt(1) },
{ actor: client3.getID()!, lamport: BigInt(4) },
]),
true,
);

// 02. Makes local changes then pull a snapshot from the server.
doc3.update((root) => {
root.t.edit(0, 0, 'c');
});
await client3.sync();
assert.equal(
versionVectorHelper(doc3.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(2001) },
{ actor: client2.getID()!, lamport: BigInt(2003) },
{ actor: client3.getID()!, lamport: BigInt(2006) },
]),
true,
);
assert.equal(
DefaultSnapshotThreshold + 2,
doc3.getRoot().t.toString().length,
);

// 03. Delete text after receiving the snapshot.
doc3.update((root) => {
root.t.edit(1, 3, '');
});
assert.equal(DefaultSnapshotThreshold, doc3.getRoot().t.toString().length);
await client3.sync();
await client2.sync();
await client1.sync();
assert.equal(DefaultSnapshotThreshold, doc2.getRoot().t.toString().length);
assert.equal(DefaultSnapshotThreshold, doc1.getRoot().t.toString().length);

await client1.deactivate();
await client2.deactivate();
await client3.deactivate();
}, 50000);
});

0 comments on commit ba70776

Please sign in to comment.