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 32 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
3 changes: 3 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ packages/sdk/src/api/yorkie/v1/resources_grpc_web_pb.d.ts
packages/sdk/src/api/yorkie/v1/resources_pb.d.ts
packages/sdk/test/vitest.d.ts
packages/sdk/lib

# examples
examples/react-tldraw/src/tldraw.d.ts
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,21 @@ and Yorkie JS SDK adheres to [Semantic Versioning](https://semver.org/spec/v2.0.

## [Unreleased]

## [0.5.1] - 2024-10-15

### Added

- Add configurable retry mechanism to broadcast interface by @gwbaik9717 in https://github.com/yorkie-team/yorkie-js-sdk/pull/901

### Changed

- Ensure `find` and `indexOf` perform splay by @m4ushold in https://github.com/yorkie-team/yorkie-js-sdk/pull/904
- Restrict presence object type to JSON serializable values by @gwbaik9717 in https://github.com/yorkie-team/yorkie-js-sdk/pull/898

### Fixed

- Automate Linting with Husky and lint-staged to Prevent CI Failures by @gwbaik9717 in https://github.com/yorkie-team/yorkie-js-sdk/pull/896

## [0.5.0] - 2024-09-05

### Added
Expand Down
6 changes: 6 additions & 0 deletions examples/react-tldraw/src/tldraw.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { Indexable, Json } from '@yorkie-js-sdk/src/document/document';
import { TDUser } from '@tldraw/tldraw';

declare module '@tldraw/tldraw' {
interface TDUser extends Indexable {}
}
2 changes: 1 addition & 1 deletion examples/vanilla-quill/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ function toDeltaOperation<T extends TextValueType>(
): DeltaOperation {
const { embed, ...restAttributes } = textValue.attributes ?? {};
if (embed) {
return { insert: JSON.parse(embed), attributes: restAttributes };
return { insert: embed, attributes: restAttributes };
}

return {
Expand Down
2 changes: 1 addition & 1 deletion examples/vanilla-quill/src/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ export type YorkieDoc = {
export type YorkiePresence = {
username: string;
color: string;
selection: TextPosStructRange | undefined;
selection?: TextPosStructRange;
};
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@
"only-allow": "^1.2.1",
"eslint-plugin-jsdoc": "^39.3.3",
"eslint-plugin-prettier": "^5.0.0"
}
},
"packageManager": "pnpm@9.9.0+sha512.60c18acd138bff695d339be6ad13f7e936eea6745660d4cc4a776d5247c540d0edee1a563695c183a66eb917ef88f2b4feb1fc25f32a7adcadc7aaf3438e99c1"
}
4 changes: 2 additions & 2 deletions packages/sdk/buf.gen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ plugins:
out: .
opt:
- target=ts
- js_import_style=module
- import_extension=none
- plugin: connect-es
out: .
opt:
- target=ts
- js_import_style=module
- import_extension=none
2 changes: 1 addition & 1 deletion packages/sdk/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "yorkie-js-sdk",
"version": "0.5.0",
"version": "0.5.1",
"description": "Yorkie JS SDK",
"main": "./src/yorkie.ts",
"publishConfig": {
Expand Down
85 changes: 69 additions & 16 deletions packages/sdk/src/api/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import Long from 'long';
import { ConnectError } from '@connectrpc/connect';
import { ErrorInfo } from '@buf/googleapis_googleapis.bufbuild_es/google/rpc/error_details_pb';
import { Code, YorkieError } from '@yorkie-js-sdk/src/util/error';
Expand Down Expand Up @@ -45,6 +44,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 +79,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 @@ -149,7 +150,7 @@ function toPresenceChange(
*/
function toCheckpoint(checkpoint: Checkpoint): PbCheckpoint {
return new PbCheckpoint({
serverSeq: checkpoint.getServerSeqAsString(),
serverSeq: checkpoint.getServerSeq(),
clientSeq: checkpoint.getClientSeq(),
});
}
Expand All @@ -160,8 +161,9 @@ function toCheckpoint(checkpoint: Checkpoint): PbCheckpoint {
function toChangeID(changeID: ChangeID): PbChangeID {
return new PbChangeID({
clientSeq: changeID.getClientSeq(),
lamport: changeID.getLamportAsString(),
lamport: changeID.getLamport(),
actorId: toUint8Array(changeID.getActorID()),
versionVector: toVersionVector(changeID.getVersionVector()),
});
}

Expand All @@ -174,12 +176,28 @@ function toTimeTicket(ticket?: TimeTicket): PbTimeTicket | undefined {
}

return new PbTimeTicket({
lamport: ticket.getLamportAsString(),
lamport: ticket.getLamport(),
delimiter: ticket.getDelimiter(),
actorId: toUint8Array(ticket.getActorID()),
});
}

/**
* `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 +799,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 All @@ -805,19 +824,34 @@ export function errorCodeOf(error: ConnectError): string {
* `fromChangeID` converts the given Protobuf format to model format.
*/
function fromChangeID(pbChangeID: PbChangeID): ChangeID {
let serverSeq: Long | undefined;
if (pbChangeID.serverSeq) {
serverSeq = Long.fromString(pbChangeID.serverSeq, true);
}

// TODO(hackerwins): Remove BigInt conversion. Some of the bigint values are
// passed as string in the protobuf. We should fix this in the future.
return ChangeID.of(
pbChangeID.clientSeq,
Long.fromString(pbChangeID.lamport, true),
BigInt(pbChangeID.lamport),
toHexString(pbChangeID.actorId),
serverSeq,
fromVersionVector(pbChangeID.versionVector)!,
JOOHOJANG marked this conversation as resolved.
Show resolved Hide resolved
BigInt(pbChangeID.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]) => {
vector.set(key, BigInt(value.toString()));
});
return vector;
}

/**
* `fromTimeTicket` converts the given Protobuf format to model format.
*/
Expand All @@ -827,7 +861,7 @@ function fromTimeTicket(pbTimeTicket?: PbTimeTicket): TimeTicket | undefined {
}

return TimeTicket.of(
Long.fromString(pbTimeTicket.lamport, true),
BigInt(pbTimeTicket.lamport),
pbTimeTicket.delimiter,
toHexString(pbTimeTicket.actorId),
);
Expand Down Expand Up @@ -1314,10 +1348,7 @@ function fromChanges<P extends Indexable>(
* `fromCheckpoint` converts the given Protobuf format to model format.
*/
function fromCheckpoint(pbCheckpoint: PbCheckpoint): Checkpoint {
return Checkpoint.of(
Long.fromString(pbCheckpoint.serverSeq, true),
pbCheckpoint.clientSeq,
);
return Checkpoint.of(BigInt(pbCheckpoint.serverSeq), pbCheckpoint.clientSeq);
}

/**
Expand All @@ -1331,6 +1362,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 +1509,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 +1660,6 @@ export const converter = {
PbChangeID,
bytesToChangeID,
bytesToOperation,
versionVectorToHex,
hexToVersionVector,
};
23 changes: 18 additions & 5 deletions 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 @@ -57,9 +58,14 @@ message Change {

message ChangeID {
uint32 client_seq = 1;
int64 server_seq = 2 [jstype = JS_STRING];
int64 lamport = 3 [jstype = JS_STRING];
int64 server_seq = 2;
int64 lamport = 3;
bytes actor_id = 4;
VersionVector version_vector = 5;
}

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

message Operation {
Expand Down Expand Up @@ -136,6 +142,12 @@ message Operation {
repeated string attributes_to_remove = 6;
map<string, TimeTicket> created_at_map_by_actor = 7;
}
message ArraySet {
TimeTicket parent_created_at = 1;
TimeTicket created_at = 2;
JSONElementSimple value = 3;
TimeTicket executed_at = 4;
}

oneof body {
Set set = 1;
Expand All @@ -148,6 +160,7 @@ message Operation {
Increase increase = 8;
TreeEdit tree_edit = 9;
TreeStyle tree_style = 10;
ArraySet array_set = 11;
}
}

Expand Down Expand Up @@ -325,7 +338,7 @@ message Presence {
}

message Checkpoint {
int64 server_seq = 1 [jstype = JS_STRING];
int64 server_seq = 1;
uint32 client_seq = 2;
}

Expand All @@ -336,7 +349,7 @@ message TextNodePos {
}

message TimeTicket {
int64 lamport = 1 [jstype = JS_STRING];
int64 lamport = 1;
uint32 delimiter = 2;
bytes actor_id = 3;
}
Expand Down
Loading