Skip to content

Commit

Permalink
Add minLamport for proper GC of deactivated clients (#1060)
Browse files Browse the repository at this point in the history
This change improves GC handling when nodes from deactivated
clients remain in the document. Previously, GC failed for such nodes because
the min version vector lacked information about deactivated clients.

The solution:
- Introduces minLamport concept for version vector comparison
- When actor info is missing in min version vector, uses minimum value instead
- Removes nodes if minimum value exceeds node's Lamport timestamp
- Ensures safe deletion by confirming all active users received the change

This approach ensures proper cleanup of orphaned nodes while maintaining data
consistency across the system.
  • Loading branch information
JOOHOJANG authored Nov 8, 2024
1 parent d22c53e commit 6547bf8
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 2 deletions.
26 changes: 24 additions & 2 deletions pkg/document/time/version_vector.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package time

import (
"bytes"
"math"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -114,7 +115,15 @@ func (v VersionVector) AfterOrEqual(other VersionVector) bool {

// EqualToOrAfter returns whether this VersionVector's every field is equal or after than given ticket.
func (v VersionVector) EqualToOrAfter(other *Ticket) bool {
return v[other.actorID.bytes] >= other.lamport
clientLamport, ok := v[other.actorID.bytes]

if !ok {
minLamport := v.MinLamport()

return minLamport > other.lamport
}

return clientLamport >= other.lamport
}

// Min returns new vv consists of every min value in each column.
Expand Down Expand Up @@ -167,7 +176,20 @@ func (v VersionVector) Max(other VersionVector) VersionVector {
return maxVV
}

// MaxLamport returns new vv consists of every max value in each column.
// MinLamport returns min lamport value in version vector.
func (v VersionVector) MinLamport() int64 {
var minLamport int64 = math.MaxInt64

for _, value := range v {
if value < minLamport {
minLamport = value
}
}

return minLamport
}

// MaxLamport returns max lamport value in version vector.
func (v VersionVector) MaxLamport() int64 {
var maxLamport int64 = -1

Expand Down
60 changes: 60 additions & 0 deletions test/integration/gc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1136,4 +1136,64 @@ func TestGarbageCollection(t *testing.T) {
assert.Equal(t, 2, d2.GarbageLen())
}
})

t.Run("gc targeting nodes made by deactivated client", func(t *testing.T) {
ctx := context.Background()
d1 := document.New(helper.TestDocKey(t))
assert.NoError(t, c1.Attach(ctx, d1))
// d2.vv =[c1:1], minvv =[c1:1], db.vv {c1: [c1:1]}
assert.Equal(t, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 1)), true)

d2 := document.New(helper.TestDocKey(t))
assert.NoError(t, c2.Attach(ctx, d2))
// d2.vv =[c1:1, c2:1], minvv =[c1:0, c2:0], db.vv {c1: [c1:1], c2: [c2:1]}
assert.Equal(t, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 1), versionOf(d2.ActorID(), 2)), true)

err := d1.Update(func(root *json.Object, p *presence.Presence) error {
root.SetNewText("text").Edit(0, 0, "a").Edit(1, 1, "b").Edit(2, 2, "c")
return nil
}, "sets text")
//d1.vv = [c1:2]
assert.Equal(t, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 2)), true)
assert.NoError(t, err)

assert.NoError(t, c1.Sync(ctx))
// d1.vv =[c1:3, c2:1], minvv =[c1:0, c2:0], db.vv {c1: [c1:2], c2: [c2:1]}
assert.Equal(t, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 3), versionOf(d2.ActorID(), 1)), true)

assert.NoError(t, c2.Sync(ctx))
// d2.vv =[c1:2, c2:3], minvv =[c1:0, c2:0], db.vv {c1: [c1:2], c2: [c1:1, c2:2]}
assert.Equal(t, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 2), versionOf(d2.ActorID(), 3)), true)

err = d2.Update(func(root *json.Object, p *presence.Presence) error {
root.GetText("text").Edit(2, 2, "c")
return nil
}, "insert c")
//d2.vv =[c1:2, c2:4]
assert.Equal(t, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 2), versionOf(d2.ActorID(), 4)), true)
assert.NoError(t, err)

err = d1.Update(func(root *json.Object, p *presence.Presence) error {
root.GetText("text").Edit(1, 3, "")
return nil
}, "delete bd")
//d1.vv = [c1:4, c2:1]
assert.Equal(t, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 1)), true)
assert.NoError(t, err)
assert.Equal(t, 2, d1.GarbageLen())
assert.Equal(t, 0, d2.GarbageLen())

assert.NoError(t, c1.Sync(ctx))
assert.NoError(t, c2.Sync(ctx))
assert.NoError(t, c1.Deactivate(ctx))

assert.Equal(t, d2.GarbageLen(), 2)
assert.Equal(t, len(d2.VersionVector()), 2)

// TODO(JOOHOJANG): remove below comments after https://github.com/yorkie-team/yorkie/issues/1058 resolved
// Due to https://github.com/yorkie-team/yorkie/issues/1058, removing deactivated client's version vector is not working properly now.
// assert.NoError(t, c2.Sync(ctx))
// assert.Equal(t, d2.GarbageLen(), 0)
// assert.Equal(t, len(d2.VersionVector()), 1)
})
}

0 comments on commit 6547bf8

Please sign in to comment.