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

Validate Checkpoint in ChangePack for PushPull API requests #959

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
495a54f
Add HTTP health check handler for server health monitoring (#952)
taeng0204 Aug 7, 2024
4344fce
Implement validation method to ensure sequential ClientSeq in reqPack…
binary-ho Aug 9, 2024
c5abdcd
Add ErrClientSeqNotSequential to errorToCode Mappers in yorkie.connect
binary-ho Aug 9, 2024
62e7f3d
Write 'packs' test code for the following cases: sequential ClientSeq…
binary-ho Aug 9, 2024
2b58d1d
Fix linting issues in packs_test
binary-ho Aug 9, 2024
2c48484
Write 'packs' test code for the following cases: sequential ClientSeq…
binary-ho Aug 10, 2024
f41ea88
Merge branch 'main' into validate-checkpoint
binary-ho Aug 10, 2024
6d13dc9
Fix goimports issues in packs_test
binary-ho Aug 10, 2024
a348192
Fix goimports issues in packs_test
binary-ho Aug 10, 2024
e447471
Rewrite test description
binary-ho Aug 10, 2024
59392e0
Fix goimports issues in packs_test.go
binary-ho Aug 10, 2024
7712bb8
Fix push/pull test where ClientSeq is less than ClientInfo's ClientSeq
binary-ho Aug 10, 2024
be485a7
Fix goimports issue at health_test.go
binary-ho Aug 10, 2024
b2472f4
Rollback health_test.go
binary-ho Aug 10, 2024
511f8f1
Add Validation ClientSeq is Sequential With DocInfo.Checkpoint.ClientSeq
binary-ho Aug 11, 2024
1917a7b
Write Test Code about validate ClientSeq is sequential with DocInfo.C…
binary-ho Aug 11, 2024
38cc7ed
Fix validate ClientSeq Sequential With Checkpoint
binary-ho Aug 11, 2024
4871d92
Remove TODO comment about needs of Changes validate
binary-ho Aug 11, 2024
a32137e
Fix lint issues: some lines too long
binary-ho Aug 11, 2024
0217951
Change to return `CodeFailedPrecondition` Status Code to the client i…
binary-ho Aug 12, 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
62 changes: 60 additions & 2 deletions server/packs/packs.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package packs

import (
"context"
"errors"
"fmt"
"strconv"
gotime "time"
Expand All @@ -37,6 +38,15 @@ import (
"github.com/yorkie-team/yorkie/server/logging"
)

var (
// ErrClientSeqNotSequentialWithCheckpoint is returned
// when ClientSeq in change not sequential With DocInfo.Checkpoint.ClientSeq
ErrClientSeqNotSequentialWithCheckpoint = errors.New("ClientSeq is not sequential with DocInfo.Checkpoint.ClientSeq")

// ErrClientSeqInChangesAreNotSequential is returned when ClientSeq in reqPack.Changes are not sequential
ErrClientSeqInChangesAreNotSequential = errors.New("ClientSeq in reqPack.Changes are not sequential")
)

// PushPullKey creates a new sync.Key of PushPull for the given document.
func PushPullKey(projectID types.ID, docKey key.Key) sync.Key {
return sync.NewKey(fmt.Sprintf("pushpull-%s-%s", projectID, docKey))
Expand Down Expand Up @@ -75,8 +85,12 @@ func PushPull(
be.Metrics.ObservePushPullResponseSeconds(gotime.Since(start).Seconds())
}()

// TODO: Changes may be reordered or missing during communication on the network.
// We should check the change.pack with checkpoint to make sure the changes are in the correct order.
checkpoint := clientInfo.Checkpoint(docInfo.ID)
err := validateClientSeqSequential(reqPack.Changes, checkpoint)
if err != nil {
return nil, err
}

initialServerSeq := docInfo.ServerSeq

// 01. push changes: filter out the changes that are already saved in the database.
Expand Down Expand Up @@ -272,3 +286,47 @@ func BuildDocumentForServerSeq(

return doc, nil
}

func validateClientSeqSequential(changes []*change.Change, checkpoint change.Checkpoint) error {
if len(changes) < 1 {
return nil
}

if err := validateClientSeqSequentialWithCheckpoint(changes, checkpoint); err != nil {
return err
}

return validateClientSeqInChangesAreSequential(changes)
}

func validateClientSeqSequentialWithCheckpoint(changes []*change.Change, checkpoint change.Checkpoint) error {
expectedClientSeq := checkpoint.ClientSeq + 1
actualFirstClientSeq := changes[0].ClientSeq()

if expectedClientSeq < actualFirstClientSeq {
return fmt.Errorf(
"ClientSeq is not sequential with DocInfo.Checkpoint.ClientSeq (expected: %d, actual: %d) : %w",
expectedClientSeq,
actualFirstClientSeq,
ErrClientSeqNotSequentialWithCheckpoint,
)
}
return nil
}

func validateClientSeqInChangesAreSequential(changes []*change.Change) error {
nextClientSeq := changes[0].ClientSeq()
for _, cn := range changes[1:] {
nextClientSeq++

if nextClientSeq != cn.ClientSeq() {
return fmt.Errorf(
"ClientSeq in Changes are not sequential (expected: %d, actual: %d) : %w",
nextClientSeq,
cn.ClientSeq(),
ErrClientSeqInChangesAreNotSequential,
)
}
}
return nil
}
Loading
Loading