Update VersionVector Operations for In-Place Comparison (#1170)

Refactored Min() and Max() functions to operate in-place, simplifying version vector comparison logic throughout the codebase. This change eliminates the need for the database.FindMinVersionVector function and streamlines version vector comparisons with direct Min() method usage.
yorkie-team · Feb 28, 2025 · d40b3b8 · d40b3b8
1 parent 23ed89a
commit d40b3b8
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 215 deletions.
diff --git a/pkg/document/change/id.go b/pkg/document/change/id.go
@@ -112,8 +112,9 @@ func (id ID) SyncClocks(other ID) ID {
 		otherVV = otherVV.DeepCopy()
 		otherVV.Set(other.actorID, other.lamport)
 	}
+	id.versionVector.Max(&otherVV)
 
-	newID := NewID(id.clientSeq, InitialServerSeq, lamport, id.actorID, id.versionVector.Max(&otherVV))
+	newID := NewID(id.clientSeq, InitialServerSeq, lamport, id.actorID, id.versionVector)
 	newID.versionVector.Set(id.actorID, lamport)
 	return newID
 }
@@ -134,8 +135,9 @@ func (id ID) SetClocks(otherLamport int64, vector time.VersionVector) ID {
 	// Semantically, including a non-client actor in version vector is
 	// problematic. To address this, we remove the InitialActorID from snapshots.
 	vector.Unset(time.InitialActorID)
+	id.versionVector.Max(&vector)
 
-	newID := NewID(id.clientSeq, id.serverSeq, lamport, id.actorID, id.versionVector.Max(&vector))
+	newID := NewID(id.clientSeq, id.serverSeq, lamport, id.actorID, id.versionVector)
 	newID.versionVector.Set(id.actorID, lamport)
 
 	return newID

diff --git a/pkg/document/time/version_vector.go b/pkg/document/time/version_vector.go
@@ -131,18 +131,12 @@ func (v VersionVector) EqualToOrAfter(other *Ticket) bool {
 }
 
 // Min modifies the receiver in-place to contain the minimum values between itself
-// and the given version vector, and returns the modified receiver.
+// and the given version vector.
 // Note: This method modifies the receiver for memory efficiency.
-func (v VersionVector) Min(other *VersionVector) VersionVector {
-	if other == nil {
-		return v
-	}
-
+func (v VersionVector) Min(other *VersionVector) {
 	for key, value := range v {
 		if otherValue, exists := (*other)[key]; exists {
-			if value < otherValue {
-				v[key] = value
-			} else {
+			if value > otherValue {
 				v[key] = otherValue
 			}
 		} else {
@@ -155,27 +149,17 @@ func (v VersionVector) Min(other *VersionVector) VersionVector {
 			v[key] = 0
 		}
 	}
-
-	return v
 }
 
 // Max modifies the receiver in-place to contain the maximum values between itself
-// and the given version vector, and returns the modified receiver.
+// and the given version vector.
 // Note: This method modifies the receiver for memory efficiency.
-func (v VersionVector) Max(other *VersionVector) VersionVector {
-	if other == nil {
-		return v
-	}
-
+func (v VersionVector) Max(other *VersionVector) {
 	for key, value := range v {
 		if otherValue, exists := (*other)[key]; exists {
-			if value > otherValue {
-				v[key] = value
-			} else {
+			if value < otherValue {
 				v[key] = otherValue
 			}
-		} else {
-			v[key] = value
 		}
 	}
 
@@ -184,8 +168,6 @@ func (v VersionVector) Max(other *VersionVector) VersionVector {
 			v[key] = value
 		}
 	}
-
-	return v
 }
 
 // MaxLamport returns max lamport value in version vector.

diff --git a/pkg/document/time/version_vector_test.go b/pkg/document/time/version_vector_test.go
@@ -101,8 +101,8 @@ func TestVersionVector(t *testing.T) {
 
 	for i, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			result := tc.v1.Min(&tests[i].v2)
-			assert.Equal(t, tc.expect, result)
+			tc.v1.Min(&tests[i].v2)
+			assert.Equal(t, tc.expect, tc.v1)
 		})
 	}
 }
diff --git a/server/backend/database/database_test.go b/server/backend/database/database_test.go
@@ -22,9 +22,6 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/yorkie-team/yorkie/api/types"
-	"github.com/yorkie-team/yorkie/pkg/document/time"
-	"github.com/yorkie-team/yorkie/server/backend/database"
-	"github.com/yorkie-team/yorkie/test/helper"
 )
 
 func TestID(t *testing.T) {
@@ -42,140 +39,3 @@ func TestID(t *testing.T) {
 		assert.Equal(t, bytes, bytesID)
 	})
 }
-
-func TestFindMinVersionVector(t *testing.T) {
-	actor1, _ := time.ActorIDFromHex("000000000000000000000001")
-	actor2, _ := time.ActorIDFromHex("000000000000000000000002")
-	actor3, _ := time.ActorIDFromHex("000000000000000000000003")
-
-	tests := []struct {
-		name            string
-		vvInfos         []database.VersionVectorInfo
-		excludeClientID types.ID
-		expect          time.VersionVector
-	}{
-		{
-			name:            "empty version vector infos",
-			vvInfos:         []database.VersionVectorInfo{},
-			excludeClientID: "",
-			expect:          nil,
-		},
-		{
-			name: "single version vector info",
-			vvInfos: []database.VersionVectorInfo{
-				{
-					ClientID: "client1",
-					VersionVector: helper.VersionVectorOf(map[*time.ActorID]int64{
-						actor1: 5,
-						actor2: 3,
-					}),
-				},
-			},
-			excludeClientID: "",
-			expect: helper.VersionVectorOf(map[*time.ActorID]int64{
-				actor1: 5,
-				actor2: 3,
-			}),
-		},
-		{
-			name: "exclude client",
-			vvInfos: []database.VersionVectorInfo{
-				{
-					ClientID: "client1",
-					VersionVector: helper.VersionVectorOf(map[*time.ActorID]int64{
-						actor1: 5,
-						actor2: 3,
-					}),
-				},
-				{
-					ClientID: "client2",
-					VersionVector: helper.VersionVectorOf(map[*time.ActorID]int64{
-						actor1: 3,
-						actor2: 4,
-					}),
-				},
-			},
-			excludeClientID: "client1",
-			expect: helper.VersionVectorOf(map[*time.ActorID]int64{
-				actor1: 3,
-				actor2: 4,
-			}),
-		},
-		{
-			name: "exclude all clients",
-			vvInfos: []database.VersionVectorInfo{
-				{
-					ClientID: "client1",
-					VersionVector: helper.VersionVectorOf(map[*time.ActorID]int64{
-						actor1: 5,
-					}),
-				},
-			},
-			excludeClientID: "client1",
-			expect:          nil,
-		},
-		{
-			name: "multiple clients with different actors",
-			vvInfos: []database.VersionVectorInfo{
-				{
-					ClientID: "client1",
-					VersionVector: helper.VersionVectorOf(map[*time.ActorID]int64{
-						actor1: 5,
-						actor2: 3,
-					}),
-				},
-				{
-					ClientID: "client2",
-					VersionVector: helper.VersionVectorOf(map[*time.ActorID]int64{
-						actor2: 2,
-						actor3: 4,
-					}),
-				},
-			},
-			excludeClientID: "",
-			expect: helper.VersionVectorOf(map[*time.ActorID]int64{
-				actor1: 0,
-				actor2: 2,
-				actor3: 0,
-			}),
-		},
-		{
-			name: "all clients have same actors",
-			vvInfos: []database.VersionVectorInfo{
-				{
-					ClientID: "client1",
-					VersionVector: helper.VersionVectorOf(map[*time.ActorID]int64{
-						actor1: 5,
-						actor2: 3,
-					}),
-				},
-				{
-					ClientID: "client2",
-					VersionVector: helper.VersionVectorOf(map[*time.ActorID]int64{
-						actor1: 3,
-						actor2: 4,
-					}),
-				},
-				{
-					ClientID: "client3",
-					VersionVector: helper.VersionVectorOf(map[*time.ActorID]int64{
-						actor1: 4,
-						actor2: 2,
-					}),
-				},
-			},
-			excludeClientID: "",
-			expect: helper.VersionVectorOf(map[*time.ActorID]int64{
-				actor1: 3,
-				actor2: 2,
-			}),
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			result := database.FindMinVersionVector(tc.vvInfos, tc.excludeClientID)
-			assert.Equal(t, tc.expect, result)
-		})
-	}
-}
diff --git a/server/backend/database/memory/database.go b/server/backend/database/memory/database.go
@@ -1366,13 +1366,13 @@ func (d *DB) UpdateAndFindMinSyncedVersionVector(
 		versionVectorInfos = append(versionVectorInfos, *vvi)
 	}
 
-	// 02-1. Compute min version vector of other clients.
-	minVersionVector := database.FindMinVersionVector(versionVectorInfos, clientInfo.ID)
-	// 02-2. Compute min version vector with current client's version vector.
-	if minVersionVector == nil {
-		minVersionVector = versionVector
-	} else {
-		minVersionVector = minVersionVector.Min(&versionVector)
+	// 02. Compute min version vector.
+	minVersionVector := versionVector.DeepCopy()
+	for i, vvi := range versionVectorInfos {
+		if vvi.ClientID == clientInfo.ID {
+			continue
+		}
+		minVersionVector.Min(&versionVectorInfos[i].VersionVector)
 	}
 
 	// 03. Update current client's version vector. If the client is detached, remove it.

diff --git a/server/backend/database/mongo/client.go b/server/backend/database/mongo/client.go
@@ -1258,13 +1258,13 @@ func (c *Client) UpdateAndFindMinSyncedVersionVector(
 		return nil, fmt.Errorf("decode version vectors: %w", err)
 	}
 
-	// 02-1. Compute min version vector of other clients.
-	minVersionVector := database.FindMinVersionVector(versionVectorInfos, clientInfo.ID)
-	// 02-2. Compute min version vector with current client's version vector.
-	if minVersionVector == nil {
-		minVersionVector = versionVector
-	} else {
-		minVersionVector = minVersionVector.Min(&versionVector)
+	// 02. Compute min version vector.
+	minVersionVector := versionVector.DeepCopy()
+	for i, vvi := range versionVectorInfos {
+		if vvi.ClientID == clientInfo.ID {
+			continue
+		}
+		minVersionVector.Min(&versionVectorInfos[i].VersionVector)
 	}
 
 	// 03. Update current client's version vector. If the client is detached, remove it.

diff --git a/server/backend/database/version_vector.go b/server/backend/database/version_vector.go
@@ -29,36 +29,3 @@ type VersionVectorInfo struct {
 	ClientID      types.ID           `bson:"client_id"`
 	VersionVector time.VersionVector `bson:"version_vector"`
 }
-
-// FindMinVersionVector finds the minimum version vector from the given version vector infos.
-// It excludes the version vector of the given client ID if specified.
-func FindMinVersionVector(vvInfos []VersionVectorInfo, excludeClientID types.ID) time.VersionVector {
-	var minVV time.VersionVector
-
-	for _, vvi := range vvInfos {
-		if vvi.ClientID == excludeClientID {
-			continue
-		}
-
-		if minVV == nil {
-			minVV = vvi.VersionVector.DeepCopy()
-			continue
-		}
-
-		for actorID, lamport := range vvi.VersionVector {
-			if currentLamport, exists := minVV[actorID]; !exists {
-				minVV[actorID] = 0
-			} else if lamport < currentLamport {
-				minVV[actorID] = lamport
-			}
-		}
-
-		for actorID := range minVV {
-			if _, exists := vvi.VersionVector[actorID]; !exists {
-				minVV[actorID] = 0
-			}
-		}
-	}
-
-	return minVV
-}