Skip to content

Commit

Permalink
Preserve Detached Client's Lamport in Version Vector (#931)
Browse files Browse the repository at this point in the history 
Following the GC error discovered in yorkie-team/yorkie#1089, we've temporarily
modified the version vector handling to retain the detached client's lamport across
local and minimum version vectors. This change provides a stopgap solution
while we investigate a more comprehensive approach to garbage collection.
  • Loading branch information
@JOOHOJANG
JOOHOJANG authored Dec 6, 2024
1 parent ea20ed9 commit 98fe2a3
Show file tree
Hide file tree
Showing 3 changed files with 201 additions and 28 deletions.
7 changes: 1 addition & 6 deletions packages/sdk/src/document/document.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1233,12 +1233,7 @@ export class Document<T, P extends Indexable = Indexable> {
this.garbageCollect(pack.getVersionVector()!);
}

// 04. Filter detached client's lamport from version vector
if (!hasSnapshot) {
this.filterVersionVector(pack.getVersionVector()!);
}

// 05. Update the status.
// 04. Update the status.
if (pack.getIsRemoved()) {
this.applyStatus(DocStatus.Removed);
}
Expand Down
22 changes: 1 addition & 21 deletions packages/sdk/src/document/time/version_vector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,26 +58,6 @@ export class VersionVector {
return max;
}

/**
* `minLamport` returns min lamport value from vector
*/
public minLamport(): bigint {
// TODO(hackerwins): If the version vector is empty, minLamport could be the
// max value of int64. This is because if the last client leaves the
// document, the min version vector becomes empty.
// This is a temporary solution and needs to be fixed later.

// 2^63-1 (int64 max)
let min = 9223372036854775807n;

for (const [, lamport] of this) {
if (lamport < min) {
min = lamport;
}
}
return min;
}

/**
* `max` returns new version vector which consists of max value of each vector
*/
Expand Down Expand Up @@ -116,7 +96,7 @@ export class VersionVector {
const lamport = this.vector.get(other.getActorID());

if (lamport === undefined) {
return this.minLamport() > other.getLamport();
return false;
}

return lamport >= other.getLamport();
Expand Down
200 changes: 199 additions & 1 deletion packages/sdk/test/integration/gc_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -601,9 +601,11 @@ describe('Garbage Collection', function () {
assert.equal(doc2.getGarbageLen(), 6);

await client1.sync();
// TODO(JOOHOJANG): we have to consider removing detached client's lamport from version vector
assert.equal(
versionVectorHelper(doc1.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(5) },
{ actor: client2.getID()!, lamport: BigInt(4) },
]),
true,
);
Expand Down Expand Up @@ -1691,7 +1693,8 @@ describe('Garbage Collection', function () {

await client2.sync();
assert.equal(doc2.getGarbageLen(), 0);
assert.equal(doc2.getVersionVector().size(), 1);
// TODO(JOOHOJANG): we have to consider removing detached client's lamport from version vector
assert.equal(doc2.getVersionVector().size(), 2);
});

it('attach > pushpull > detach lifecycle version vector test (run gc at last client detaches document, but no tombstone exsits)', async function ({
Expand Down Expand Up @@ -1836,6 +1839,7 @@ describe('Garbage Collection', function () {
await client2.sync();
assert.equal(
versionVectorHelper(doc2.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(8) },
{ actor: client2.getID()!, lamport: BigInt(9) },
]),
true,
Expand All @@ -1846,6 +1850,7 @@ describe('Garbage Collection', function () {
}, 'delete all');
assert.equal(
versionVectorHelper(doc2.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(8) },
{ actor: client2.getID()!, lamport: BigInt(10) },
]),
true,
Expand All @@ -1854,6 +1859,7 @@ describe('Garbage Collection', function () {
await client2.sync();
assert.equal(
versionVectorHelper(doc2.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(8) },
{ actor: client2.getID()!, lamport: BigInt(10) },
]),
true,
Expand Down Expand Up @@ -1995,6 +2001,7 @@ describe('Garbage Collection', function () {
assert.equal(doc2.getGarbageLen(), 0);

await client1.sync();
// TODO(JOOHOJANG): we have to consider removing detached client's lamport from version vector
assert.equal(
versionVectorHelper(doc1.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(7) },
Expand All @@ -2007,6 +2014,7 @@ describe('Garbage Collection', function () {
await client2.sync();
assert.equal(
versionVectorHelper(doc2.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(8) },
{ actor: client2.getID()!, lamport: BigInt(9) },
]),
true,
Expand All @@ -2017,6 +2025,7 @@ describe('Garbage Collection', function () {
}, 'delete all');
assert.equal(
versionVectorHelper(doc2.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(8) },
{ actor: client2.getID()!, lamport: BigInt(10) },
]),
true,
Expand All @@ -2028,4 +2037,193 @@ describe('Garbage Collection', function () {
await client1.deactivate();
await client2.deactivate();
});

it('detach gc 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 });

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

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

doc1.update((root) => {
root.t = new Text();
root.t.edit(0, 0, 'a');
root.t.edit(1, 1, 'b');
root.t.edit(2, 2, 'c');
}, '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(4) },
{ actor: client2.getID()!, lamport: BigInt(5) },
{ actor: client3.getID()!, lamport: BigInt(1) },
]),
true,
);
assert.equal(
versionVectorHelper(doc3.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(4) },
{ actor: client2.getID()!, lamport: BigInt(1) },
{ actor: client3.getID()!, lamport: BigInt(5) },
]),
true,
);

doc3.update((root) => {
root.t.edit(1, 3, '');
});

doc1.update((root) => {
root.t.edit(0, 0, '1');
});
doc1.update((root) => {
root.t.edit(0, 0, '2');
});
doc1.update((root) => {
root.t.edit(0, 0, '3');
});
doc2.update((root) => {
root.t.edit(3, 3, 'x');
});
doc2.update((root) => {
root.t.edit(4, 4, 'y');
});

await client1.sync();
await client2.sync();
await client1.sync();
assert.equal(
doc1.toJSON(),
'{"t":[{"val":"3"},{"val":"2"},{"val":"1"},{"val":"a"},{"val":"b"},{"val":"c"},{"val":"x"},{"val":"y"}]}',
);
assert.equal(
doc2.toJSON(),
'{"t":[{"val":"3"},{"val":"2"},{"val":"1"},{"val":"a"},{"val":"b"},{"val":"c"},{"val":"x"},{"val":"y"}]}',
);

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

await client3.detach(doc3);
doc2.update((root) => {
root.t.edit(5, 5, 'z');
});

await client1.sync();
assert.equal(doc1.getGarbageLen(), 2);
await client1.sync();
assert.equal(doc1.getGarbageLen(), 2);

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

// TODO(JOOHOJANG): we have to consider removing detached client's lamport from version vector
assert.equal(
versionVectorHelper(doc1.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(12) },
{ actor: client2.getID()!, lamport: BigInt(11) },
{ actor: client3.getID()!, lamport: BigInt(7) },
]),
true,
);
assert.equal(
versionVectorHelper(doc2.getVersionVector(), [
{ actor: client1.getID()!, lamport: BigInt(7) },
{ actor: client2.getID()!, lamport: BigInt(13) },
{ actor: client3.getID()!, lamport: BigInt(7) },
]),
true,
);
assert.equal(
doc1.toJSON(),
'{"t":[{"val":"3"},{"val":"2"},{"val":"1"},{"val":"a"},{"val":"z"},{"val":"x"},{"val":"y"}]}',
);
assert.equal(
doc2.toJSON(),
'{"t":[{"val":"3"},{"val":"2"},{"val":"1"},{"val":"a"},{"val":"z"},{"val":"x"},{"val":"y"}]}',
);
assert.equal(doc1.getGarbageLen(), 2);
assert.equal(doc2.getGarbageLen(), 2);
await client2.sync();
await client1.sync();
assert.equal(doc1.getGarbageLen(), 0);
assert.equal(doc2.getGarbageLen(), 0);

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

0 comments on commit 98fe2a3

Please sign in to comment.