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

Introduce VersionVector #1047

Merged
merged 15 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
6 changes: 6 additions & 0 deletions admin/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,10 +329,16 @@ func (c *Client) ListChangeSummaries(
return nil, err
}

vector, err := converter.FromVersionVector(snapshotMeta.Msg.VersionVector)
if err != nil {
return nil, err
}

newDoc, err := document.NewInternalDocumentFromSnapshot(
key,
seq,
snapshotMeta.Msg.Lamport,
vector,
snapshotMeta.Msg.Snapshot,
)

Expand Down
40 changes: 37 additions & 3 deletions api/converter/from_pb.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,19 +97,27 @@ func FromChangePack(pbPack *api.ChangePack) (*change.Pack, error) {
return nil, err
}

versionVector, err := FromVersionVector(pbPack.VersionVector)
if err != nil {
return nil, err
}

minSyncedTicket, err := fromTimeTicket(pbPack.MinSyncedTicket)
if err != nil {
return nil, err
}

return &change.Pack{
pack := &change.Pack{
DocumentKey: key.Key(pbPack.DocumentKey),
Checkpoint: fromCheckpoint(pbPack.Checkpoint),
Changes: changes,
Snapshot: pbPack.Snapshot,
MinSyncedTicket: minSyncedTicket,
IsRemoved: pbPack.IsRemoved,
}, nil
VersionVector: versionVector,
MinSyncedTicket: minSyncedTicket,
}

return pack, nil
}

func fromCheckpoint(pbCheckpoint *api.Checkpoint) change.Checkpoint {
Expand Down Expand Up @@ -147,14 +155,40 @@ func fromChangeID(id *api.ChangeID) (change.ID, error) {
if err != nil {
return change.InitialID, err
}

vector, err := FromVersionVector(id.VersionVector)
if err != nil {
return change.InitialID, err
}

return change.NewID(
id.ClientSeq,
id.ServerSeq,
id.Lamport,
actorID,
vector,
), nil
}

// FromVersionVector converts the given Protobuf formats to model format.
func FromVersionVector(pbVersionVector *api.VersionVector) (time.VersionVector, error) {
versionVector := make(time.VersionVector)
// TODO(hackerwins): Old clients do not send VersionVector. We should remove this later.
if pbVersionVector == nil {
return versionVector, nil
}

for id, lamport := range pbVersionVector.Vector {
actorID, err := time.ActorIDFromHex(id)
if err != nil {
return nil, err
}
versionVector.Set(actorID, lamport)
}

return versionVector, nil
}

// FromDocumentID converts the given Protobuf formats to model format.
func FromDocumentID(pbID string) (types.ID, error) {
id := types.ID(pbID)
Expand Down
45 changes: 39 additions & 6 deletions api/converter/to_pb.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,18 @@ func ToChangePack(pack *change.Pack) (*api.ChangePack, error) {
return nil, err
}

pbVersionVector, err := ToVersionVector(pack.VersionVector)
if err != nil {
return nil, err
}

return &api.ChangePack{
DocumentKey: pack.DocumentKey.String(),
Checkpoint: ToCheckpoint(pack.Checkpoint),
Changes: pbChanges,
Snapshot: pack.Snapshot,
MinSyncedTicket: ToTimeTicket(pack.MinSyncedTicket),
VersionVector: pbVersionVector,
IsRemoved: pack.IsRemoved,
}, nil
}
Expand All @@ -155,13 +161,35 @@ func ToCheckpoint(cp change.Checkpoint) *api.Checkpoint {
}

// ToChangeID converts the given model format to Protobuf format.
func ToChangeID(id change.ID) *api.ChangeID {
func ToChangeID(id change.ID) (*api.ChangeID, error) {
pbVersionVector, err := ToVersionVector(id.VersionVector())
if err != nil {
return nil, err
}
return &api.ChangeID{
ClientSeq: id.ClientSeq(),
ServerSeq: id.ServerSeq(),
Lamport: id.Lamport(),
ActorId: id.ActorID().Bytes(),
ClientSeq: id.ClientSeq(),
ServerSeq: id.ServerSeq(),
Lamport: id.Lamport(),
ActorId: id.ActorID().Bytes(),
VersionVector: pbVersionVector,
}, nil
}

// ToVersionVector converts the given model format to Protobuf format.
func ToVersionVector(vector time.VersionVector) (*api.VersionVector, error) {
pbVersionVector := make(map[string]int64)
for actor, clock := range vector {
id, err := time.ActorIDFromBytes(actor[:])
if err != nil {
return nil, err
}

pbVersionVector[id.String()] = clock
}

return &api.VersionVector{
Vector: pbVersionVector,
}, nil
}

// ToDocEventType converts the given model format to Protobuf format.
Expand Down Expand Up @@ -243,8 +271,13 @@ func ToChanges(changes []*change.Change) ([]*api.Change, error) {
return nil, err
}

pbChangeID, err := ToChangeID(c.ID())
if err != nil {
return nil, err
}

pbChanges = append(pbChanges, &api.Change{
Id: ToChangeID(c.ID()),
Id: pbChangeID,
Message: c.Message(),
Operations: pbOperations,
PresenceChange: ToPresenceChange(c.PresenceChange()),
Expand Down
41 changes: 41 additions & 0 deletions api/docs/yorkie/v1/admin.openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,12 @@ components:
- type: string
- type: number
title: server_seq
versionVector:
$ref: '#/components/schemas/yorkie.v1.VersionVector'
additionalProperties: false
description: ""
title: version_vector
type: object
title: ChangeID
type: object
yorkie.v1.ChangePasswordRequest:
Expand Down Expand Up @@ -964,6 +970,12 @@ components:
format: byte
title: snapshot
type: string
versionVector:
$ref: '#/components/schemas/yorkie.v1.VersionVector'
additionalProperties: false
description: ""
title: version_vector
type: object
title: GetSnapshotMetaResponse
type: object
yorkie.v1.JSONElementSimple:
Expand Down Expand Up @@ -2225,6 +2237,35 @@ components:
- 13
title: ValueType
type: string
yorkie.v1.VersionVector:
additionalProperties: false
description: ""
properties:
vector:
additionalProperties: false
description: ""
title: vector
type: object
title: VersionVector
type: object
yorkie.v1.VersionVector.VectorEntry:
additionalProperties: false
description: ""
properties:
key:
additionalProperties: false
description: ""
title: key
type: string
value:
additionalProperties: false
description: ""
oneOf:
- type: string
- type: number
title: value
title: VectorEntry
type: object
securitySchemes:
ApiKeyAuth:
in: header
Expand Down
43 changes: 42 additions & 1 deletion api/docs/yorkie/v1/resources.openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,12 @@ components:
- type: string
- type: number
title: server_seq
versionVector:
$ref: '#/components/schemas/yorkie.v1.VersionVector'
additionalProperties: false
description: ""
title: version_vector
type: object
title: ChangeID
type: object
yorkie.v1.ChangePack:
Expand Down Expand Up @@ -253,7 +259,7 @@ components:
minSyncedTicket:
$ref: '#/components/schemas/yorkie.v1.TimeTicket'
additionalProperties: false
description: ""
description: Deprecated
title: min_synced_ticket
type: object
snapshot:
Expand All @@ -262,6 +268,12 @@ components:
format: byte
title: snapshot
type: string
versionVector:
$ref: '#/components/schemas/yorkie.v1.VersionVector'
additionalProperties: false
description: ""
title: version_vector
type: object
title: ChangePack
type: object
yorkie.v1.Checkpoint:
Expand Down Expand Up @@ -1751,6 +1763,35 @@ components:
- 13
title: ValueType
type: string
yorkie.v1.VersionVector:
additionalProperties: false
description: ""
properties:
vector:
additionalProperties: false
description: ""
title: vector
type: object
title: VersionVector
type: object
yorkie.v1.VersionVector.VectorEntry:
additionalProperties: false
description: ""
properties:
key:
additionalProperties: false
description: ""
title: key
type: string
value:
additionalProperties: false
description: ""
oneOf:
- type: string
- type: number
title: value
title: VectorEntry
type: object
securitySchemes:
ApiKeyAuth:
in: header
Expand Down
43 changes: 42 additions & 1 deletion api/docs/yorkie/v1/yorkie.openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,12 @@ components:
- type: string
- type: number
title: server_seq
versionVector:
$ref: '#/components/schemas/yorkie.v1.VersionVector'
additionalProperties: false
description: ""
title: version_vector
type: object
Comment on lines +429 to +434
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

Based on the search results, I can now generate the final response since we have enough information about both Lamport clock and version vector usage in the API specs.

Both Lamport clock and version vector fields are present without deprecation notices

The search results show that both lamport and versionVector fields are currently present in the API specs without any deprecation notices. Since both fields exist simultaneously:

  • The lamport field appears in multiple API spec files without any deprecation notice
  • The versionVector field is also present, suggesting a transition period
  • Neither field has proper documentation in their description fields

To maintain API clarity during this transition:

  • Add deprecation notice to the lamport field
  • Add proper description for the versionVector field explaining its role as the replacement
  • Consider documenting the migration timeline in the API specs
🔗 Analysis chain

Verify deprecation of Lamport clock fields.

Since the PR aims to replace Lamport clocks with version vectors, consider marking the lamport field in the ChangeID schema as deprecated.

Add deprecation notice:

         lamport:
           additionalProperties: false
-          description: ""
+          description: "Deprecated: This field is being replaced by versionVector for improved distributed causality tracking."
           oneOf:
           - type: string
           - type: number
           title: lamport

Let me try a different search strategy to verify the Lamport clock usage and deprecation status.

Let's also check for version vector related changes to understand the transition:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if Lamport clock is still being used in the codebase
echo "Searching for Lamport clock usage..."
rg -l "lamport" --type proto

Length of output: 141


Script:

#!/bin/bash
# Search for Lamport clock usage in yaml files and any other relevant files
echo "Searching for Lamport clock usage in API specs..."
rg -l "lamport" --type yaml

# Also check the actual field definition
echo -e "\nChecking the ChangeID schema definition..."
rg "lamport:" -A 5 api/docs/yorkie/v1/yorkie.openapi.yaml

Length of output: 713


Script:

#!/bin/bash
# Search for version vector related definitions
echo "Searching for version vector related definitions..."
rg "versionVector" -A 5 api/docs/yorkie/v1/yorkie.openapi.yaml

Length of output: 588

title: ChangeID
type: object
yorkie.v1.ChangePack:
Expand Down Expand Up @@ -461,7 +467,7 @@ components:
minSyncedTicket:
$ref: '#/components/schemas/yorkie.v1.TimeTicket'
additionalProperties: false
description: ""
description: Deprecated
title: min_synced_ticket
type: object
snapshot:
Expand All @@ -470,6 +476,12 @@ components:
format: byte
title: snapshot
type: string
versionVector:
$ref: '#/components/schemas/yorkie.v1.VersionVector'
additionalProperties: false
description: ""
title: version_vector
type: object
title: ChangePack
type: object
yorkie.v1.Checkpoint:
Expand Down Expand Up @@ -1548,6 +1560,35 @@ components:
- 13
title: ValueType
type: string
yorkie.v1.VersionVector:
additionalProperties: false
description: ""
properties:
vector:
additionalProperties: false
description: ""
title: vector
type: object
title: VersionVector
type: object
yorkie.v1.VersionVector.VectorEntry:
additionalProperties: false
description: ""
properties:
key:
additionalProperties: false
description: ""
title: key
type: string
value:
additionalProperties: false
description: ""
oneOf:
- type: string
- type: number
title: value
title: VectorEntry
type: object
yorkie.v1.WatchDocumentRequest:
additionalProperties: false
description: ""
Expand Down
Loading
Loading