From d07ce0230a3b9c9e8cfa0f4af4815550dcc73814 Mon Sep 17 00:00:00 2001 From: Seungyong Lee Date: Mon, 2 Sep 2024 23:47:48 +0900 Subject: [PATCH] Optimize FindChangeInfosBetweenServerSeqs to prevent unnecessary Query (#974) This commit modifies the FindChangeInfosBetweenServerSeqs method to avoid executing DB queries when the from > to. This change minimizes unnecessary database resource consumption, particularly in scenarios where the most recent document editor, User A, continues editing without any interference from other users (B, C, etc.). In such cases, in PushPull, User A's Checkpoint.serverSeq always equal to the server's initialServerSeq (DocInfo.serverSeq), leading to a situation where from > to in the FindChangeInfosBetweenServerSeqs. By preventing the execution of a query under these circumstances, we can reduce the load on database resources. --- server/backend/database/memory/database.go | 3 + .../backend/database/memory/database_test.go | 4 + server/backend/database/mongo/client.go | 3 + server/backend/database/mongo/client_test.go | 4 + .../backend/database/testcases/testcases.go | 179 ++++++++++++++++++ test/sharding/mongo_client_test.go | 4 + 6 files changed, 197 insertions(+) diff --git a/server/backend/database/memory/database.go b/server/backend/database/memory/database.go index f778c5e6a..13a04789c 100644 --- a/server/backend/database/memory/database.go +++ b/server/backend/database/memory/database.go @@ -1052,6 +1052,9 @@ func (d *DB) FindChangeInfosBetweenServerSeqs( txn := d.db.Txn(false) defer txn.Abort() + if from > to { + return nil, nil + } var infos []*database.ChangeInfo iterator, err := txn.LowerBound( diff --git a/server/backend/database/memory/database_test.go b/server/backend/database/memory/database_test.go index 5ee6ad0a1..d308940a7 100644 --- a/server/backend/database/memory/database_test.go +++ b/server/backend/database/memory/database_test.go @@ -60,6 +60,10 @@ func TestDB(t *testing.T) { testcases.RunFindChangesBetweenServerSeqsTest(t, db, projectID) }) + t.Run("RunFindChangeInfosBetweenServerSeqsTest test", func(t *testing.T) { + testcases.RunFindChangeInfosBetweenServerSeqsTest(t, db, projectID) + }) + t.Run("RunFindClosestSnapshotInfo test", func(t *testing.T) { testcases.RunFindClosestSnapshotInfoTest(t, db, projectID) }) diff --git a/server/backend/database/mongo/client.go b/server/backend/database/mongo/client.go index 7e8daca79..16ec85b63 100644 --- a/server/backend/database/mongo/client.go +++ b/server/backend/database/mongo/client.go @@ -1031,6 +1031,9 @@ func (c *Client) FindChangeInfosBetweenServerSeqs( from int64, to int64, ) ([]*database.ChangeInfo, error) { + if from > to { + return nil, nil + } cursor, err := c.collection(ColChanges).Find(ctx, bson.M{ "project_id": docRefKey.ProjectID, "doc_id": docRefKey.DocID, diff --git a/server/backend/database/mongo/client_test.go b/server/backend/database/mongo/client_test.go index 03e596840..6e0803b5d 100644 --- a/server/backend/database/mongo/client_test.go +++ b/server/backend/database/mongo/client_test.go @@ -76,6 +76,10 @@ func TestClient(t *testing.T) { testcases.RunFindChangesBetweenServerSeqsTest(t, cli, dummyProjectID) }) + t.Run("RunFindChangeInfosBetweenServerSeqsTest test", func(t *testing.T) { + testcases.RunFindChangeInfosBetweenServerSeqsTest(t, cli, dummyProjectID) + }) + t.Run("RunFindClosestSnapshotInfo test", func(t *testing.T) { testcases.RunFindClosestSnapshotInfoTest(t, cli, dummyProjectID) }) diff --git a/server/backend/database/testcases/testcases.go b/server/backend/database/testcases/testcases.go index 1dc9212e9..2108f8c83 100644 --- a/server/backend/database/testcases/testcases.go +++ b/server/backend/database/testcases/testcases.go @@ -343,6 +343,185 @@ func RunFindChangesBetweenServerSeqsTest( }) } +// RunFindChangeInfosBetweenServerSeqsTest runs the FindChangeInfosBetweenServerSeqs test for the given db. +func RunFindChangeInfosBetweenServerSeqsTest( + t *testing.T, + db database.Database, + projectID types.ID, +) { + t.Run("continues editing without any interference from other users test", func(t *testing.T) { + ctx := context.Background() + + docKey := key.Key(fmt.Sprintf("tests$%s", t.Name())) + + clientInfo, _ := db.ActivateClient(ctx, projectID, t.Name()) + docInfo, _ := db.FindDocInfoByKeyAndOwner(ctx, clientInfo.RefKey(), docKey, true) + assert.NoError(t, clientInfo.AttachDocument(docInfo.ID, false)) + assert.NoError(t, db.UpdateClientInfoAfterPushPull(ctx, clientInfo, docInfo)) + + updatedClientInfo, _ := db.FindClientInfoByRefKey(ctx, clientInfo.RefKey()) + + // Record the serverSeq value at the time the PushPull request came in. + initialServerSeq := docInfo.ServerSeq + + // The serverSeq of the checkpoint that the server has should always be the same as + // the serverSeq of the user's checkpoint that came in as a request, if no other user interfered. + reqPackCheckpointServerSeq := updatedClientInfo.Checkpoint(docInfo.ID).ServerSeq + + changeInfos, err := db.FindChangeInfosBetweenServerSeqs( + ctx, + docInfo.RefKey(), + reqPackCheckpointServerSeq+1, + initialServerSeq, + ) + + assert.NoError(t, err) + assert.Len(t, changeInfos, 0) + }) + + t.Run("retrieving a document with snapshot that reflect the latest doc info test", func(t *testing.T) { + ctx := context.Background() + + docKey := key.Key(fmt.Sprintf("tests$%s", t.Name())) + + clientInfo, _ := db.ActivateClient(ctx, projectID, t.Name()) + docInfo, _ := db.FindDocInfoByKeyAndOwner(ctx, clientInfo.RefKey(), docKey, true) + docRefKey := docInfo.RefKey() + assert.NoError(t, clientInfo.AttachDocument(docInfo.ID, false)) + assert.NoError(t, db.UpdateClientInfoAfterPushPull(ctx, clientInfo, docInfo)) + + initialServerSeq := docInfo.ServerSeq + + // 01. Create a document and store changes + bytesID, _ := clientInfo.ID.Bytes() + actorID, _ := time.ActorIDFromBytes(bytesID) + doc := document.New(key.Key(t.Name())) + doc.SetActor(actorID) + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.SetNewArray("array") + return nil + })) + for idx := 0; idx < 5; idx++ { + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.GetArray("array").AddInteger(idx) + return nil + })) + } + + pack := doc.CreateChangePack() + for _, c := range pack.Changes { + serverSeq := docInfo.IncreaseServerSeq() + c.SetServerSeq(serverSeq) + } + + err := db.CreateChangeInfos( + ctx, + projectID, + docInfo, + initialServerSeq, + pack.Changes, + false, + ) + assert.NoError(t, err) + + // 02. Create a snapshot that reflect the latest doc info + updatedDocInfo, _ := db.FindDocInfoByRefKey(ctx, docRefKey) + assert.Equal(t, int64(6), updatedDocInfo.ServerSeq) + + pack = change.NewPack( + updatedDocInfo.Key, + change.InitialCheckpoint.NextServerSeq(updatedDocInfo.ServerSeq), + nil, + nil, + ) + assert.NoError(t, doc.ApplyChangePack(pack)) + assert.Equal(t, int64(6), doc.Checkpoint().ServerSeq) + + assert.NoError(t, db.CreateSnapshotInfo(ctx, docRefKey, doc.InternalDocument())) + + // 03. Find changeInfos with snapshot that reflect the latest doc info + snapshotInfo, _ := db.FindClosestSnapshotInfo( + ctx, + docRefKey, + updatedDocInfo.ServerSeq, + false, + ) + + changeInfos, _ := db.FindChangeInfosBetweenServerSeqs( + ctx, + docRefKey, + snapshotInfo.ServerSeq+1, + updatedDocInfo.ServerSeq, + ) + + assert.Len(t, changeInfos, 0) + }) + + t.Run("store changes and find changes test", func(t *testing.T) { + ctx := context.Background() + + docKey := key.Key(fmt.Sprintf("tests$%s", t.Name())) + + clientInfo, _ := db.ActivateClient(ctx, projectID, t.Name()) + docInfo, _ := db.FindDocInfoByKeyAndOwner(ctx, clientInfo.RefKey(), docKey, true) + docRefKey := docInfo.RefKey() + assert.NoError(t, clientInfo.AttachDocument(docInfo.ID, false)) + assert.NoError(t, db.UpdateClientInfoAfterPushPull(ctx, clientInfo, docInfo)) + + initialServerSeq := docInfo.ServerSeq + + // 01. Create a document and store changes + bytesID, _ := clientInfo.ID.Bytes() + actorID, _ := time.ActorIDFromBytes(bytesID) + doc := document.New(key.Key(t.Name())) + doc.SetActor(actorID) + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.SetNewArray("array") + return nil + })) + for idx := 0; idx < 5; idx++ { + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.GetArray("array").AddInteger(idx) + return nil + })) + } + pack := doc.CreateChangePack() + for _, c := range pack.Changes { + serverSeq := docInfo.IncreaseServerSeq() + c.SetServerSeq(serverSeq) + } + + err := db.CreateChangeInfos( + ctx, + projectID, + docInfo, + initialServerSeq, + pack.Changes, + false, + ) + assert.NoError(t, err) + + // 02. Find changes + changeInfos, err := db.FindChangeInfosBetweenServerSeqs( + ctx, + docRefKey, + 1, + 6, + ) + assert.NoError(t, err) + assert.Len(t, changeInfos, 6) + + changeInfos, err = db.FindChangeInfosBetweenServerSeqs( + ctx, + docRefKey, + 3, + 3, + ) + assert.NoError(t, err) + assert.Len(t, changeInfos, 1) + }) +} + // RunFindClosestSnapshotInfoTest runs the FindClosestSnapshotInfo test for the given db. func RunFindClosestSnapshotInfoTest(t *testing.T, db database.Database, projectID types.ID) { t.Run("store and find snapshots test", func(t *testing.T) { diff --git a/test/sharding/mongo_client_test.go b/test/sharding/mongo_client_test.go index 0653e0200..5cb2bffe6 100644 --- a/test/sharding/mongo_client_test.go +++ b/test/sharding/mongo_client_test.go @@ -85,6 +85,10 @@ func TestClientWithShardedDB(t *testing.T) { testcases.RunFindChangesBetweenServerSeqsTest(t, cli, dummyProjectID) }) + t.Run("RunFindChangeInfosBetweenServerSeqsTest test", func(t *testing.T) { + testcases.RunFindChangeInfosBetweenServerSeqsTest(t, cli, dummyProjectID) + }) + t.Run("RunFindClosestSnapshotInfo test", func(t *testing.T) { testcases.RunFindClosestSnapshotInfoTest(t, cli, dummyProjectID) })