Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introducing version vector to solve GC problem #899

Merged
merged 38 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
e0988d0
Update Proto
JOOHOJANG Sep 8, 2024
9580b97
Update converter to use version vector
JOOHOJANG Sep 8, 2024
dee2778
Update Document to handle version vector in applychangepack
JOOHOJANG Sep 8, 2024
f6177d2
Define version vector and add into change_id, changepack
JOOHOJANG Sep 8, 2024
df82b83
Update GC in CRDT Root to use min synced version vector
JOOHOJANG Sep 8, 2024
0dd1ece
Update test helper to compute MaxVersionVector
JOOHOJANG Sep 8, 2024
11d4848
Update TC to handle version vector
JOOHOJANG Sep 8, 2024
313a81c
Define version vector getter in document
JOOHOJANG Sep 8, 2024
cadecd8
Fix typo
JOOHOJANG Sep 8, 2024
f4e3431
Define version vector helper in test
JOOHOJANG Sep 8, 2024
3e3afe7
Modify GC test to compare version vector
JOOHOJANG Sep 8, 2024
557702e
Disable empty arrow function lint error
JOOHOJANG Sep 9, 2024
6f383e6
Add GC test for concurrent case and concurrent pushonly case
JOOHOJANG Sep 9, 2024
13f528b
Update bench test
JOOHOJANG Sep 9, 2024
0c6a7db
Temporarily add unload event code to detach user when close/refresh tab
JOOHOJANG Sep 10, 2024
f4f7ff4
Fix lint error
JOOHOJANG Sep 12, 2024
40d5792
Modify buf generate
JOOHOJANG Sep 24, 2024
cae3f9c
Remove snapshot version vector
JOOHOJANG Oct 1, 2024
ee8e411
Remove min synced version vector from proto
JOOHOJANG Oct 1, 2024
e3b438e
Modify min synced version vector related logics
JOOHOJANG Oct 1, 2024
6f14089
Test modifications based on the implementation approach
JOOHOJANG Oct 1, 2024
95e5cac
Remove TODO
JOOHOJANG Oct 22, 2024
047510f
Restrict presence object type to JSON serializable values (#898)
gwbaik9717 Sep 12, 2024
95e8871
Add configurable retry mechanism to broadcast interface (#901)
gwbaik9717 Sep 30, 2024
9f670ad
Ensure `find` and `indexOf` perform splay (#904)
m4ushold Sep 30, 2024
6e98529
Update CHANGELOG.md for v0.5.1 (#910)
hackerwins Oct 15, 2024
6e7e344
Update target to ES2020 and replace Long with bigint (#912)
hackerwins Oct 21, 2024
6376dbf
Define version vector and add into change_id, changepack
JOOHOJANG Sep 8, 2024
06569de
Modify buf generate
JOOHOJANG Sep 24, 2024
73aad7b
Remove snapshot version vector
JOOHOJANG Oct 1, 2024
4dc3e1c
Remove min synced version vector from proto
JOOHOJANG Oct 1, 2024
3aff0b5
Modify min synced version vector related logics
JOOHOJANG Oct 1, 2024
43b2f6b
Merge branch 'main' into hybrid-clock-version-vector-modified
JOOHOJANG Oct 22, 2024
7e39737
Remove Long from helper
JOOHOJANG Oct 22, 2024
3528aa0
Clean up codes
JOOHOJANG Oct 22, 2024
e5321c4
Clean up codes
JOOHOJANG Oct 22, 2024
6d5ece1
Clean up codes
JOOHOJANG Oct 22, 2024
c4dea81
Clean up codes
JOOHOJANG Oct 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions examples/vanilla-codemirror6/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,18 @@ async function main() {
await client.activate();

// 02-1. create a document then attach it into the client.
const doc = new yorkie.Document<YorkieDoc>(
`codemirror6-${new Date()
.toISOString()
.substring(0, 10)
.replace(/-/g, '')}`,
{
enableDevtools: true,
},
);
// TODO(JOOHOJANG):Put it back to how the document’s dockey was.
const doc = new yorkie.Document<YorkieDoc>('dailysync', {
enableDevtools: true,
});
window.addEventListener('beforeunload', async () => {
await client.detach(doc);
await client.deactivate();
});
window.addEventListener('unload', async () => {
await client.detach(doc);
await client.deactivate();
});
doc.subscribe('connection', (event) => {
network.statusListener(networkStatusElem)(event);
});
Expand Down
4 changes: 2 additions & 2 deletions packages/sdk/buf.gen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ plugins:
- plugin: es
out: .
opt:
- target=ts
- target=js+dts
- js_import_style=module
- plugin: connect-es
out: .
opt:
- target=ts
- target=js+dts
- js_import_style=module
61 changes: 61 additions & 0 deletions packages/sdk/src/api/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { RGATreeList } from '@yorkie-js-sdk/src/document/crdt/rga_tree_list';
import { CRDTElement } from '@yorkie-js-sdk/src/document/crdt/element';
import { CRDTObject } from '@yorkie-js-sdk/src/document/crdt/object';
import { CRDTArray } from '@yorkie-js-sdk/src/document/crdt/array';
import { VersionVector } from '@yorkie-js-sdk/src/document/time/version_vector';
import { CRDTTreePos } from './../document/crdt/tree';
import {
RGATreeSplit,
Expand Down Expand Up @@ -79,6 +80,7 @@ import {
TreeNodes as PbTreeNodes,
TreePos as PbTreePos,
TreeNodeID as PbTreeNodeID,
VersionVector as PbVersionVector,
ValueType as PbValueType,
JSONElement_Tree as PbJSONElement_Tree,
JSONElement_Text as PbJSONElement_Text,
Expand Down Expand Up @@ -162,6 +164,7 @@ function toChangeID(changeID: ChangeID): PbChangeID {
clientSeq: changeID.getClientSeq(),
lamport: changeID.getLamportAsString(),
actorId: toUint8Array(changeID.getActorID()),
versionVector: toVersionVector(changeID.getVersionVector()),
});
}

Expand All @@ -180,6 +183,22 @@ function toTimeTicket(ticket?: TimeTicket): PbTimeTicket | undefined {
});
}

/**
* `toVersionVector` converts the given model to Protobuf format.
*/
function toVersionVector(vector: VersionVector): PbVersionVector | undefined {
if (!vector) {
return;
}

const pbVector = new PbVersionVector();
for (const [actorID, lamport] of vector) {
// TODO(hackerwins): Remove Long after introducing BigInt.
pbVector.vector[actorID] = BigInt(lamport.toString());
}
return pbVector;
}
JOOHOJANG marked this conversation as resolved.
Show resolved Hide resolved

/**
* `toValueType` converts the given model to Protobuf format.
*/
Expand Down Expand Up @@ -781,6 +800,7 @@ function toChangePack(pack: ChangePack<Indexable>): PbChangePack {
isRemoved: pack.getIsRemoved(),
changes: toChanges(pack.getChanges()),
snapshot: pack.getSnapshot(),
versionVector: toVersionVector(pack.getVersionVector()!),
JOOHOJANG marked this conversation as resolved.
Show resolved Hide resolved
minSyncedTicket: toTimeTicket(pack.getMinSyncedTicket()),
});
}
Expand Down Expand Up @@ -814,10 +834,29 @@ function fromChangeID(pbChangeID: PbChangeID): ChangeID {
pbChangeID.clientSeq,
Long.fromString(pbChangeID.lamport, true),
toHexString(pbChangeID.actorId),
fromVersionVector(pbChangeID.versionVector)!,
JOOHOJANG marked this conversation as resolved.
Show resolved Hide resolved
serverSeq,
);
}

/**
* `fromVersionVector` converts the given Protobuf format to model format.
*/
function fromVersionVector(
pbVersionVector?: PbVersionVector,
): VersionVector | undefined {
if (!pbVersionVector) {
return;
}

const vector = new VersionVector();
Object.entries(pbVersionVector.vector).forEach(([key, value]) => {
// TODO(hackerwins): Remove Long after introducing BigInt.
vector.set(key, Long.fromString(value.toString(), true));
});
return vector;
}
JOOHOJANG marked this conversation as resolved.
Show resolved Hide resolved

/**
* `fromTimeTicket` converts the given Protobuf format to model format.
*/
Expand Down Expand Up @@ -1331,6 +1370,7 @@ function fromChangePack<P extends Indexable>(
fromCheckpoint(pbPack.checkpoint!),
pbPack.isRemoved,
fromChanges(pbPack.changes),
fromVersionVector(pbPack.versionVector),
pbPack.snapshot,
fromTimeTicket(pbPack.minSyncedTicket),
);
Expand Down Expand Up @@ -1477,6 +1517,25 @@ function bytesToSnapshot<P extends Indexable>(
};
}

/**
* `versionVectorToHex` converts the given VersionVector to bytes.
*/
function versionVectorToHex(vector: VersionVector): string {
const pbVersionVector = toVersionVector(vector)!;

return bytesToHex(pbVersionVector.toBinary());
}
JOOHOJANG marked this conversation as resolved.
Show resolved Hide resolved

/**
* `hexToVersionVector` creates a VersionVector from the given bytes.
*/
function hexToVersionVector(hex: string): VersionVector {
const bytes = hexToBytes(hex);
const pbVersionVector = PbVersionVector.fromBinary(bytes);

return fromVersionVector(pbVersionVector)!;
}
Comment on lines +1527 to +1528
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid Unnecessary Non-Null Assertion Operator in hexToVersionVector

In the function hexToVersionVector, the non-null assertion operator ! is used in fromVersionVector(pbVersionVector)!. Since fromVersionVector can return undefined, using ! here could result in a runtime error if fromVersionVector returns undefined. It's advisable to handle the potential undefined value or adjust fromVersionVector to guarantee a valid VersionVector is returned.


/**
* `bytesToObject` creates an JSONObject from the given byte array.
*/
Expand Down Expand Up @@ -1609,4 +1668,6 @@ export const converter = {
PbChangeID,
bytesToChangeID,
bytesToOperation,
versionVectorToHex,
hexToVersionVector,
};
8 changes: 7 additions & 1 deletion packages/sdk/src/api/yorkie/v1/resources.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ message ChangePack {
Checkpoint checkpoint = 2;
bytes snapshot = 3;
repeated Change changes = 4;
TimeTicket min_synced_ticket = 5;
TimeTicket min_synced_ticket = 5; // Deprecated
bool is_removed = 6;
VersionVector version_vector = 7;
}

message Change {
Expand All @@ -60,6 +61,11 @@ message ChangeID {
int64 server_seq = 2 [jstype = JS_STRING];
int64 lamport = 3 [jstype = JS_STRING];
bytes actor_id = 4;
VersionVector version_vector = 5;
}

message VersionVector {
map<string, int64> vector = 1;
}

message Operation {
Expand Down
38 changes: 37 additions & 1 deletion packages/sdk/src/api/yorkie/v1/resources_pb.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// @generated by protoc-gen-es v1.6.0 with parameter "target=js+dts,js_import_style=module"
// @generated by protoc-gen-es v1.10.0 with parameter "target=js+dts,js_import_style=module"
// @generated from file src/api/yorkie/v1/resources.proto (package yorkie.v1, syntax proto3)
/* eslint-disable */
// @ts-nocheck
Expand Down Expand Up @@ -182,6 +182,8 @@ export declare class ChangePack extends Message<ChangePack> {
changes: Change[];

/**
* Deprecated
*
JOOHOJANG marked this conversation as resolved.
Show resolved Hide resolved
* @generated from field: yorkie.v1.TimeTicket min_synced_ticket = 5;
*/
minSyncedTicket?: TimeTicket;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Review usage of deprecated minSyncedTicket field.

The minSyncedTicket field is marked as deprecated. Consider removing this field if it is no longer needed, and ensure that all references to it are updated to use the new versionVector field or appropriate alternative.

Expand All @@ -191,6 +193,11 @@ export declare class ChangePack extends Message<ChangePack> {
*/
isRemoved: boolean;

/**
* @generated from field: yorkie.v1.VersionVector version_vector = 7;
*/
versionVector?: VersionVector;

JOOHOJANG marked this conversation as resolved.
Show resolved Hide resolved
constructor(data?: PartialMessage<ChangePack>);

static readonly runtime: typeof proto3;
Expand Down Expand Up @@ -269,6 +276,11 @@ export declare class ChangeID extends Message<ChangeID> {
*/
actorId: Uint8Array;

/**
* @generated from field: yorkie.v1.VersionVector version_vector = 5;
*/
versionVector?: VersionVector;

constructor(data?: PartialMessage<ChangeID>);

static readonly runtime: typeof proto3;
Expand All @@ -284,6 +296,30 @@ export declare class ChangeID extends Message<ChangeID> {
static equals(a: ChangeID | PlainMessage<ChangeID> | undefined, b: ChangeID | PlainMessage<ChangeID> | undefined): boolean;
}

/**
* @generated from message yorkie.v1.VersionVector
*/
export declare class VersionVector extends Message<VersionVector> {
/**
* @generated from field: map<string, int64> vector = 1;
*/
vector: { [key: string]: bigint };

constructor(data?: PartialMessage<VersionVector>);

static readonly runtime: typeof proto3;
static readonly typeName = "yorkie.v1.VersionVector";
static readonly fields: FieldList;

static fromBinary(bytes: Uint8Array, options?: Partial<BinaryReadOptions>): VersionVector;

static fromJson(jsonValue: JsonValue, options?: Partial<JsonReadOptions>): VersionVector;

static fromJsonString(jsonString: string, options?: Partial<JsonReadOptions>): VersionVector;

static equals(a: VersionVector | PlainMessage<VersionVector> | undefined, b: VersionVector | PlainMessage<VersionVector> | undefined): boolean;
}
JOOHOJANG marked this conversation as resolved.
Show resolved Hide resolved

/**
* @generated from message yorkie.v1.Operation
*/
Expand Down
Loading
Loading