Skip to content

Commit aed75bf

Browse files
gregns1gregns1
committed
address commnets
1 parent 1a91b03 commit aed75bf

File tree

6 files changed

+531
-409
lines changed

6 files changed

+531
-409
lines changed

‎db/blip_sync_context.go

Lines changed: 78 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,6 @@ func (bsc *BlipSyncContext) handleChangesResponse(ctx context.Context, sender *b
351351
if err != nil {
352352
return err
353353
}
354-
versionVectorProtocol := bsc.useHLV()
355354

356355
for i, knownRevsArrayInterface := range answer {
357356
seq := changeArray[i][0].(SequenceID)
@@ -367,55 +366,23 @@ func (bsc *BlipSyncContext) handleChangesResponse(ctx context.Context, sender *b
367366
knownRevsByDoc[docID] = knownRevs
368367
}
369368

370-
// The first element of the knownRevsArray returned from CBL is the parent revision to use as deltaSrc for
371-
// revtree clients. For HLV clients, use the cv as deltaSrc
372-
if bsc.useDeltas && len(knownRevsArray) > 0 {
373-
if revID, ok := knownRevsArray[0].(string); ok {
374-
if versionVectorProtocol {
375-
// extract cv from the known revs array
376-
msgHLV, _, err := extractHLVFromBlipString(revID)
377-
if err != nil {
378-
base.DebugfCtx(ctx, base.KeySync, "Invalid known rev format for hlv on doc: %s falling back to full body replication.", base.UD(docID))
379-
deltaSrcRevID = "" // will force falling back to full body replication below
380-
} else {
381-
deltaSrcRevID = msgHLV.GetCurrentVersionString()
382-
}
383-
} else {
384-
deltaSrcRevID = revID
385-
}
386-
}
387-
}
388-
389-
var cvInKnownRevs bool
390-
391-
for _, rev := range knownRevsArray {
392-
if revID, ok := rev.(string); ok {
393-
// extract cv from the known revs array
394-
msgHLV, _, err := extractHLVFromBlipString(revID)
395-
if err != nil {
396-
// assume we have received legacy rev if the following conditions are met:
397-
// - we cannot parse cv from known revs
398-
// - we are in vv enabled replication
399-
// - the first element of known rev array was NOT a CV
400-
if versionVectorProtocol {
401-
if !cvInKnownRevs {
402-
// if cv wasn't the first element in known revs array, this will be legacy doc
403-
legacyRev = true
404-
}
405-
}
406-
} else {
407-
// extract cv as string
408-
revID = msgHLV.GetCurrentVersionString()
409-
// mark that we have seen a CV in known revs so we can mark as non legacy rev
410-
cvInKnownRevs = true
411-
}
412-
// we can assume here that if we fail to parse hlv, we have received a rev id in known revs. If we don't fail to parse hlv
413-
// then we have extracted cv from it and can assign the cv string to known revs here
414-
knownRevs[revID] = true
415-
} else {
416-
base.ErrorfCtx(ctx, "Invalid response to 'changes' message")
417-
return nil
418-
}
369+
// known revs can be in the following format for subprotocol version < 4:
370+
// - [] indicating the document is not present on the client
371+
// - [revID, ...] if the client wants the revision but has a revision of this document already. In this
372+
// case the client will send its leaf nodes of its rev tree.
373+
// For subprotocol version > 4 known revs can be in the following format:
374+
// - [] indicating the document is not present on the client
375+
// - [revID] indicating the clients wants this doc but already has a revision and this revision is a pre
376+
// upgraded revision (a document without a HLV yet as it hasn't been updated since upgrade)
377+
// ISGR ONLY:
378+
// - [cv, revID] indicating the clients wants this doc but already has a revision and this revision. First
379+
// element must always be a CV here which will be used for delta sync if enabled. The subsequent revID
380+
// will be used to determine what revIDs will need to be included in the revTree property for ISGR
381+
382+
deltaSrcRevID, legacyRev, knownRevs, err = bsc.getKnownRevs(ctx, docID, knownRevsArray)
383+
if err != nil {
384+
base.ErrorfCtx(ctx, "Invalid response to 'changes' message")
385+
return nil
419386
}
420387

421388
var err error
@@ -880,3 +847,64 @@ func (bsc *BlipSyncContext) useHLV() bool {
880847
func (bsc *BlipSyncContext) sendRevTreeProperty() bool {
881848
return bsc.activeCBMobileSubprotocol >= CBMobileReplicationV4 && bsc.clientType == BLIPClientTypeSGR2
882849
}
850+
851+
// getKnownRevs will return deltaSrcRev (if delta sync is enabled), whether the known rev array represents a legacy
852+
// document, the parsed known revs in a map and error if unknown format is received.
853+
func (bsc *BlipSyncContext) getKnownRevs(ctx context.Context, docID string, knownRevsArray []interface{}) (deltaSrcRev string, changeIsLegacyRev bool, knownRevs map[string]bool, err error) {
854+
knownRevs = make(map[string]bool)
855+
// The first element of the knownRevsArray returned from CBL is the parent revision to use as deltaSrc for
856+
// revtree clients. For HLV clients, use the cv as deltaSrc
857+
if bsc.useDeltas && len(knownRevsArray) > 0 {
858+
if revID, ok := knownRevsArray[0].(string); ok {
859+
if bsc.useHLV() {
860+
// extract cv from the known revs array
861+
msgHLV, _, err := extractHLVFromBlipString(revID)
862+
if err != nil {
863+
base.DebugfCtx(ctx, base.KeySync, "Invalid known rev format for hlv on doc: %s falling back to full body replication.", base.UD(docID))
864+
deltaSrcRev = "" // will force falling back to full body replication below
865+
} else {
866+
deltaSrcRev = msgHLV.GetCurrentVersionString()
867+
}
868+
} else {
869+
deltaSrcRev = revID
870+
}
871+
}
872+
}
873+
874+
var cvInKnownRevs bool
875+
for _, rev := range knownRevsArray {
876+
if revID, ok := rev.(string); ok {
877+
// extract cv from the known revs array
878+
msgHLV, _, err := extractHLVFromBlipString(revID)
879+
if err != nil {
880+
// assume we have received legacy rev if the following conditions are met:
881+
// - we cannot parse cv from known revs
882+
// - we are in vv enabled replication
883+
// - the first element of known rev array was NOT a CV
884+
if bsc.useHLV() {
885+
if !cvInKnownRevs {
886+
// if cv wasn't the first element in known revs array, this will be legacy doc
887+
changeIsLegacyRev = true
888+
}
889+
}
890+
} else {
891+
// extract cv as string
892+
revID = msgHLV.GetCurrentVersionString()
893+
// mark that we have seen a CV in known revs so we can mark as non legacy rev
894+
cvInKnownRevs = true
895+
}
896+
// we can assume here that if we fail to parse hlv, we have received a rev id in known revs. If we don't fail to parse hlv
897+
// then we have extracted cv from it and can assign the cv string to known revs here
898+
knownRevs[revID] = true
899+
} else {
900+
return deltaSrcRev, changeIsLegacyRev, knownRevs, errors.New("invalid known rev format")
901+
}
902+
}
903+
904+
if cvInKnownRevs && len(knownRevsArray) > 2 {
905+
// invalid format received
906+
return deltaSrcRev, changeIsLegacyRev, knownRevs, errors.New("invalid known rev format")
907+
}
908+
909+
return deltaSrcRev, changeIsLegacyRev, knownRevs, nil
910+
}

‎db/blip_sync_context_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"github.com/couchbase/sync_gateway/base"
1717
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
1819
)
1920

2021
// TestBlipSyncContextSetUseDeltas verifies all permutations of setUseDeltas()
@@ -101,3 +102,72 @@ func BenchmarkBlipSyncContextSetUseDeltas(b *testing.B) {
101102
})
102103
}
103104
}
105+
106+
func TestKnownRevs(t *testing.T) {
107+
testCases := []struct {
108+
name string
109+
knownRevs []interface{}
110+
expectedKnown []string
111+
isLegacyRev bool
112+
legacyProtocol bool
113+
errorCase bool
114+
}{
115+
{
116+
name: "no known revs",
117+
knownRevs: []interface{}{},
118+
expectedKnown: []string{},
119+
},
120+
{
121+
name: "< 4 subprotocol no known revs",
122+
knownRevs: []interface{}{},
123+
legacyProtocol: true,
124+
expectedKnown: []string{},
125+
},
126+
{
127+
name: "< 4 subprotocol: client has known rev",
128+
knownRevs: []interface{}{"1-abc"},
129+
expectedKnown: []string{"1-abc"},
130+
legacyProtocol: true,
131+
},
132+
{
133+
name: ">= 4 subprotocol: client has known rev",
134+
knownRevs: []interface{}{"123@src", "1-abc"},
135+
expectedKnown: []string{"123@src", "1-abc"},
136+
},
137+
{
138+
name: ">= 4 subprotocol: client has known rev and rev is legacy rev",
139+
knownRevs: []interface{}{"1-abc"},
140+
expectedKnown: []string{"1-abc"},
141+
isLegacyRev: true,
142+
},
143+
{
144+
name: "invalid known revs",
145+
knownRevs: []interface{}{"123@src1", "1-abc", "123@src2"},
146+
errorCase: true,
147+
},
148+
}
149+
for _, tc := range testCases {
150+
t.Run(tc.name, func(t *testing.T) {
151+
testCtx := t.Context()
152+
var bsc *BlipSyncContext
153+
if tc.legacyProtocol {
154+
bsc = &BlipSyncContext{
155+
activeCBMobileSubprotocol: CBMobileReplicationV3,
156+
}
157+
} else {
158+
bsc = &BlipSyncContext{
159+
activeCBMobileSubprotocol: CBMobileReplicationV4,
160+
}
161+
}
162+
163+
_, changeLegacy, knownRevisions, err := bsc.getKnownRevs(testCtx, "someID", tc.knownRevs)
164+
if !tc.errorCase {
165+
require.NoError(t, err)
166+
assert.Equal(t, tc.isLegacyRev, changeLegacy)
167+
base.RequireKeysEqual(t, tc.expectedKnown, knownRevisions)
168+
} else {
169+
require.Error(t, err)
170+
}
171+
})
172+
}
173+
}

‎db/crud.go

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,7 +1211,15 @@ func (db *DatabaseCollectionWithUser) Put(ctx context.Context, docid string, bod
12111211
return newRevID, doc, err
12121212
}
12131213

1214-
func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Context, newDoc *Document, newDocHLV *HybridLogicalVector, existingDoc *sgbucket.BucketDocument, revTreeHistory []string, revTreeISGRProperty bool) (doc *Document, cv *Version, newRevID string, err error) {
1214+
// PutExistingCurrentVersion:
1215+
// - newDoc: new incoming doc
1216+
// - newDocHLV: new incoming doc's HLV
1217+
// - existsingDoc: existing doc in bucket (if present)
1218+
// - revTreeHistory: list of revID's from the incoming docs history (including docs current rev).
1219+
// - alignRevTrees: if this is true then we will align the new write with the incoming docs rev tree. If this is
1220+
// false and len(revTreeHistory) > 0 then this meaans the lcoal version of this doc is legacy doc so this parameter
1221+
// will be used to check for conflicts.
1222+
func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Context, newDoc *Document, newDocHLV *HybridLogicalVector, existingDoc *sgbucket.BucketDocument, revTreeHistory []string, alignRevTrees bool) (doc *Document, cv *Version, newRevID string, err error) {
12151223
var matchRev string
12161224
if existingDoc != nil {
12171225
doc, unmarshalErr := db.unmarshalDocumentWithXattrs(ctx, newDoc.ID, existingDoc.Body, existingDoc.Xattrs, existingDoc.Cas, DocUnmarshalRev)
@@ -1265,7 +1273,7 @@ func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Cont
12651273
// if incoming rev tree list is from a legacy pre upgraded doc, we should have new revID generation based
12661274
// off the previous current rev +1. If we have rev tree list filled from ISGR's rev tree property then we
12671275
// should use the current rev of inc
1268-
if !revTreeISGRProperty {
1276+
if !alignRevTrees {
12691277
newGeneration = prevGeneration + 1
12701278
} else {
12711279
newGeneration = prevGeneration
@@ -1283,7 +1291,7 @@ func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Cont
12831291
if addNewerVersionsErr != nil {
12841292
return nil, nil, false, nil, addNewerVersionsErr
12851293
}
1286-
if revTreeISGRProperty {
1294+
if alignRevTrees {
12871295
err = doc.alignRevTreeHistory(ctx, newDoc, revTreeHistory)
12881296
if err != nil {
12891297
return nil, nil, false, nil, err
@@ -1302,7 +1310,7 @@ func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Cont
13021310
}
13031311
// the new document has a dominating hlv, so we can ignore any legacy rev revtree information on the incoming document
13041312
revTreeConflictChecked = true
1305-
if !revTreeISGRProperty {
1313+
if !alignRevTrees {
13061314
previousRevTreeID = doc.CurrentRev
13071315
} else {
13081316
// align rev tree here for ISGR replications
@@ -1313,14 +1321,14 @@ func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Cont
13131321
}
13141322
} else {
13151323
// if the legacy rev list is from ISGR property, we should not do a conflict check
1316-
if len(revTreeHistory) > 0 && !revTreeISGRProperty {
1324+
if len(revTreeHistory) > 0 && !alignRevTrees {
13171325
// conflict check on rev tree history, if there is a rev in rev tree history we have the parent of locally we are not in conflict
13181326
parent, currentRevIndex, err = db.revTreeConflictCheck(ctx, revTreeHistory, doc, newDoc.Deleted)
13191327
if err != nil {
13201328
base.InfofCtx(ctx, base.KeyCRUD, "conflict detected between the two HLV's for doc %s, incoming version %s, local version %s, and conflict found in rev tree history", base.UD(doc.ID), newDocHLV.GetCurrentVersionString(), doc.HLV.GetCurrentVersionString())
13211329
return nil, nil, false, nil, err
13221330
}
1323-
err = doc.addNewerRevisionsToHistory(newDoc, currentRevIndex, parent, revTreeHistory)
1331+
_, err = doc.addNewerRevisionsToRevTreeHistory(newDoc, currentRevIndex, parent, revTreeHistory)
13241332
if err != nil {
13251333
return nil, nil, false, nil, err
13261334
}
@@ -1342,13 +1350,15 @@ func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Cont
13421350
}
13431351
// rev tree conflict check if we have rev tree history to check against + finds current rev index to allow us
13441352
// to add any new revision to rev tree below.
1345-
// Only check for rev tree conflicts if we haven't already checked above
1346-
if !revTreeConflictChecked && len(revTreeHistory) > 0 && !revTreeISGRProperty {
1353+
// Only check for rev tree conflicts if we haven't already checked above AND if alignRevTrees is false given
1354+
// alignRevTrees can only be true for non legacy rev writes meaning when this is true we should be doing our
1355+
// conflict checking with incoming HLV
1356+
if !revTreeConflictChecked && len(revTreeHistory) > 0 && !alignRevTrees {
13471357
parent, currentRevIndex, err = db.revTreeConflictCheck(ctx, revTreeHistory, doc, newDoc.Deleted)
13481358
if err != nil {
13491359
return nil, nil, false, nil, err
13501360
}
1351-
err = doc.addNewerRevisionsToHistory(newDoc, currentRevIndex, parent, revTreeHistory)
1361+
_, err = doc.addNewerRevisionsToRevTreeHistory(newDoc, currentRevIndex, parent, revTreeHistory)
13521362
if err != nil {
13531363
return nil, nil, false, nil, err
13541364
}
@@ -1362,7 +1372,7 @@ func (db *DatabaseCollectionWithUser) PutExistingCurrentVersion(ctx context.Cont
13621372

13631373
// generate rev id for new arriving doc
13641374
var newRev string
1365-
if !revTreeISGRProperty {
1375+
if !alignRevTrees {
13661376
// create a new revID for incoming write
13671377
strippedBody, _ := stripInternalProperties(newDoc._body)
13681378
encoding, err := base.JSONMarshalCanonical(strippedBody)
@@ -3366,6 +3376,9 @@ func (db *DatabaseCollectionWithUser) CheckProposedVersion(ctx context.Context,
33663376
}
33673377
}
33683378

3379+
// alignRevTreeHistory will take incoming rev tree list and add any newer revisions to the local document. If there is
3380+
// history between the incoming rev tree and local rev tree differs then the local docs rev tree will be overwritten to
3381+
// the incoming rev tree.
33693382
func (doc *Document) alignRevTreeHistory(ctx context.Context, newDoc *Document, revTreeHistory []string) error {
33703383
currentRevIndex := len(revTreeHistory)
33713384
parent := ""
@@ -3381,30 +3394,36 @@ func (doc *Document) alignRevTreeHistory(ctx context.Context, newDoc *Document,
33813394
base.DebugfCtx(ctx, base.KeyCRUD, "incoming rev tree history has different history than local doc %s, aligning local doc history with incoming rev tree history", base.UD(doc.ID))
33823395
// clean local history and make way for incoming history to replace it
33833396
doc.History = make(RevTree)
3397+
// reset current rev index and parent given we are building a new rev tree now
3398+
currentRevIndex = len(revTreeHistory)
3399+
parent = ""
33843400
}
33853401

3386-
err := doc.addNewerRevisionsToHistory(newDoc, currentRevIndex, parent, revTreeHistory)
3402+
newRev, err := doc.addNewerRevisionsToRevTreeHistory(newDoc, currentRevIndex, parent, revTreeHistory)
33873403
if err != nil {
33883404
return err
33893405
}
3406+
doc.CurrentRev = newRev
33903407
return nil
33913408
}
33923409

3393-
func (doc *Document) addNewerRevisionsToHistory(newDoc *Document, currentRevIndex int, parent string, docHistory []string) error {
3410+
// addNewerRevisionsToRevTreeHistory will add any newer rev tree id's to the local document history
3411+
func (doc *Document) addNewerRevisionsToRevTreeHistory(newDoc *Document, currentRevIndex int, parent string, docHistory []string) (string, error) {
3412+
// currentRevIndex here is the index of the incoming rev tree list to start from.
33943413
for i := currentRevIndex - 1; i >= 0; i-- {
33953414
err := doc.History.addRevision(newDoc.ID,
33963415
RevInfo{
33973416
ID: docHistory[i],
3398-
Parent: parent,
3417+
Parent: parent, // set the parent of this revision to the element of docHistory from the last iteration
33993418
Deleted: i == 0 && newDoc.Deleted})
34003419

34013420
if err != nil {
3402-
return err
3421+
return "", err
34033422
}
34043423
parent = docHistory[i]
34053424
}
3406-
doc.CurrentRev = parent
3407-
return nil
3425+
// return last element added from docHistory, this will be the doc's new current rev for writes that are aligning rev tree
3426+
return parent, nil
34083427
}
34093428

34103429
const (

0 commit comments

Comments
 (0)