Skip to content

Commit 2b9ed17

Browse files
gregns1gregns1
authored
CBG-5106: Incorrect history being sent for legacy revision cases in delta sync (#8307)
1 parent 342c231 commit 2b9ed17

5 files changed

Lines changed: 371 additions & 76 deletions

File tree

‎db/blip_handler.go‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ func (bh *blipHandler) handleProposeChanges(rq *blip.Message) error {
913913

914914
// ////// DOCUMENTS:
915915

916-
func (bsc *BlipSyncContext) sendRevAsDelta(ctx context.Context, sender *blip.Sender, docID, revID string, deltaSrcRevID string, seq SequenceID, knownRevs map[string]bool, maxHistory int, handleChangesResponseCollection *DatabaseCollectionWithUser, collectionIdx *int) error {
916+
func (bsc *BlipSyncContext) sendRevAsDelta(ctx context.Context, sender *blip.Sender, docID, revID string, deltaSrcRevID string, seq SequenceID, knownRevs map[string]bool, maxHistory int, handleChangesResponseCollection *DatabaseCollectionWithUser, collectionIdx *int, remoteIsLegacyRev bool) error {
917917
bsc.replicationStats.SendRevDeltaRequestedCount.Add(1)
918918

919919
revDelta, redactedRev, err := handleChangesResponseCollection.GetDelta(ctx, docID, deltaSrcRevID, revID)
@@ -922,40 +922,36 @@ func (bsc *BlipSyncContext) sendRevAsDelta(ctx context.Context, sender *blip.Sen
922922
} else if base.IsFleeceDeltaError(err) {
923923
// Something went wrong in the diffing library. We want to know about this!
924924
base.WarnfCtx(ctx, "Falling back to full body replication. Error generating delta from %s to %s for key %s - err: %v", deltaSrcRevID, revID, base.UD(docID), err)
925-
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, false)
925+
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, remoteIsLegacyRev)
926926
} else if err == base.ErrDeltaSourceIsTombstone {
927927
base.TracefCtx(ctx, base.KeySync, "Falling back to full body replication. Delta source %s is tombstone. Unable to generate delta to %s for key %s", deltaSrcRevID, revID, base.UD(docID))
928-
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, false)
928+
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, remoteIsLegacyRev)
929929
} else if err != nil {
930930
base.DebugfCtx(ctx, base.KeySync, "Falling back to full body replication. Couldn't get delta from %s to %s for key %s - err: %v", deltaSrcRevID, revID, base.UD(docID), err)
931-
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, false)
931+
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, remoteIsLegacyRev)
932932
}
933933

934934
if redactedRev != nil {
935-
var history []string
936-
var revTreeProperty []string
937-
if !bsc.useHLV() || bsc.sendRevTreeProperty() {
935+
localIsLegacyRev := redactedRev.CV == nil
936+
var revTreeHistory []string
937+
if !bsc.useHLV() || localIsLegacyRev || remoteIsLegacyRev || bsc.sendRevTreeProperty() {
938938
var err error
939-
history, err = toHistory(redactedRev.History, knownRevs, maxHistory)
939+
revTreeHistory, err = toHistory(redactedRev.History, knownRevs, maxHistory)
940940
if err != nil {
941941
err := base.RedactErrorf("Could not get rev tree history for replacement rev in sendRevAsDelta %s %s: %w, sending a noRev to skip this revision for replication at sequence %s.", base.UD(docID), revID, err, seq)
942942
base.WarnfCtx(ctx, "%s", err)
943943
return bsc.sendNoRev(sender, docID, revID, collectionIdx, seq, err)
944944
}
945-
} else {
946-
history = append(history, redactedRev.HlvHistory)
947-
}
948-
if bsc.sendRevTreeProperty() {
949-
revTreeProperty = append(revTreeProperty, redactedRev.RevID)
950-
history, err := toHistory(redactedRev.History, knownRevs, maxHistory)
951-
if err != nil {
952-
err := base.RedactErrorf("Could not get rev tree history for replacement rev when sending in sendRevAsDelta %s %s: %w, sending a noRev to skip this document for replication at sequence %s.", base.UD(docID), redactedRev.RevID, err, seq)
953-
base.WarnfCtx(ctx, "%s", err)
954-
return bsc.sendNoRev(sender, docID, revID, collectionIdx, seq, err)
955-
}
956-
revTreeProperty = append(revTreeProperty, history...)
957945
}
958946

947+
history, revTreeProperty := bsc.buildRevHistory(revHistoryInput{
948+
revTreeHistory: revTreeHistory,
949+
hlvHistory: redactedRev.HlvHistory,
950+
revID: redactedRev.RevID,
951+
localIsLegacyRev: localIsLegacyRev,
952+
remoteIsLegacyRev: remoteIsLegacyRev,
953+
})
954+
959955
properties, err := blipRevMessageProperties(history, redactedRev.Deleted, seq, "", revTreeProperty)
960956
if err != nil {
961957
return err
@@ -965,16 +961,16 @@ func (bsc *BlipSyncContext) sendRevAsDelta(ctx context.Context, sender *blip.Sen
965961

966962
if revDelta == nil {
967963
base.DebugfCtx(ctx, base.KeySync, "Falling back to full body replication. Couldn't get delta from %s to %s for key %s", deltaSrcRevID, revID, base.UD(docID))
968-
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, false)
964+
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, remoteIsLegacyRev)
969965
}
970966

971967
resendFullRevisionFunc := func() error {
972968
base.InfofCtx(ctx, base.KeySync, "Resending revision as full body. Peer couldn't process delta %s from %s to %s for key %s", base.UD(revDelta.DeltaBytes), deltaSrcRevID, revID, base.UD(docID))
973-
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, false)
969+
return bsc.sendRevision(ctx, sender, docID, revID, seq, knownRevs, maxHistory, handleChangesResponseCollection, collectionIdx, remoteIsLegacyRev)
974970
}
975971

976972
base.TracefCtx(ctx, base.KeySync, "docID: %s - delta: %v", base.UD(docID), base.UD(string(revDelta.DeltaBytes)))
977-
if err := bsc.sendDelta(ctx, sender, docID, collectionIdx, deltaSrcRevID, revDelta, seq, resendFullRevisionFunc); err != nil {
973+
if err := bsc.sendDelta(ctx, sender, docID, collectionIdx, deltaSrcRevID, revDelta, seq, remoteIsLegacyRev, resendFullRevisionFunc); err != nil {
978974
return err
979975
}
980976

‎db/blip_sync_context.go‎

Lines changed: 111 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ func (bsc *BlipSyncContext) handleChangesResponse(ctx context.Context, sender *b
393393
var err error
394394

395395
if deltaSrcRevID != "" {
396-
err = bsc.sendRevAsDelta(ctx, sender, docID, rev, deltaSrcRevID, seq, knownRevs, maxHistory, handleChangesResponseDbCollection, collectionIdx)
396+
err = bsc.sendRevAsDelta(ctx, sender, docID, rev, deltaSrcRevID, seq, knownRevs, maxHistory, handleChangesResponseDbCollection, collectionIdx, legacyRev)
397397
} else {
398398
err = bsc.sendRevision(ctx, sender, docID, rev, seq, knownRevs, maxHistory, handleChangesResponseDbCollection, collectionIdx, legacyRev)
399399
}
@@ -585,21 +585,109 @@ func (bsc *BlipSyncContext) setUseDeltas(clientCanUseDeltas bool) {
585585
}
586586
}
587587

588-
func (bsc *BlipSyncContext) sendDelta(ctx context.Context, sender *blip.Sender, docID string, collectionIdx *int, deltaSrcRevID string, revDelta *RevisionDelta, seq SequenceID, resendFullRevisionFunc func() error) error {
588+
// revHistoryInput contains the inputs needed to build revision history properties for a BLIP rev message.
589+
type revHistoryInput struct {
590+
revTreeHistory []string // Pre-computed rev tree ancestor chain (from toHistory or RevisionDelta.RevisionHistory)
591+
hlvHistory string // HLV history string in CBL format
592+
revID string // Current document's rev tree ID (e.g. "2-abc")
593+
localIsLegacyRev bool // True if the local revision predates HLV (no version vector yet)
594+
remoteIsLegacyRev bool // True if the remote peer's version of this doc predates HLV
595+
}
589596

590-
var history []string
591-
var revTreeProperty []string
592-
// CV can now be empty on rev cache items; only use CV when it's available
593-
// history being sent here is potentially incorrect pending CBG-5106
594-
if bsc.useHLV() && revDelta.ToCV != "" {
595-
history = append(history, revDelta.HlvHistory)
597+
// buildRevHistory constructs the "history" and "revTreeHistory" BLIP message properties for a rev
598+
// message. These properties tell the peer what version history the document has, so the peer can
599+
// determine ancestry and detect conflicts.
600+
//
601+
// A "legacy" revision is one that predates HLV (Hybrid Logical Vector) — it has a rev tree ID
602+
// (e.g. "2-abc") but no version vector. After a node upgrades, new writes get both a rev tree ID
603+
// and a CV (current version). The combination of local/remote legacy status produces four scenarios:
604+
//
605+
// Scenario 1: Pre-HLV protocol (subprotocol < V4) or post-HLV protocol (subprotocol >= V4) and both sides have legacy rev
606+
// ┌─────────────────────────────────────────────────────┐
607+
// │ history property: [revTreeHistory...] │
608+
// │ revTreeHistory property: (not sent) │
609+
// │ │
610+
// │ Both sides only understand rev trees or only have │
611+
// │ rev trees, so send the rev tree ancestor chain │
612+
// │ in the history property. │
613+
// └─────────────────────────────────────────────────────┘
614+
//
615+
// Scenario 2: Local has HLV, remote has HLV (CBL client)
616+
// ┌─────────────────────────────────────────────────────┐
617+
// │ history property: [hlvHistory] │
618+
// │ revTreeHistory prop: (not sent) │
619+
// │ │
620+
// │ Both sides speak HLV. Send only the HLV history. │
621+
// │ CBL clients don't need the rev tree property. │
622+
// └─────────────────────────────────────────────────────┘
623+
//
624+
// Scenario 3: Local has HLV, remote is legacy
625+
// ┌─────────────────────────────────────────────────────┐
626+
// │ history property: [hlvHistory, revID, │
627+
// │ revTreeHistory...] │
628+
// │ revTreeHistory prop: (not sent) │
629+
// │ │
630+
// │ The remote has a legacy rev tree version. Append │
631+
// │ the current revID + rev tree history after the HLV │
632+
// │ history so the remote can detect conflicts via its │
633+
// │ rev tree. │
634+
// └─────────────────────────────────────────────────────┘
635+
//
636+
// Scenario 4: Local legacy, remote has HLV
637+
// ┌─────────────────────────────────────────────────────┐
638+
// │ history property: [revTreeHistory...] │
639+
// │ revTreeHistory prop: (not sent) │
640+
// │ │
641+
// │ The remote has a HLV version. Assign the rev tree │
642+
// │ to history so the remote can parse for conflict │
643+
// │ checks. │
644+
// └─────────────────────────────────────────────────────┘
645+
//
646+
// Scenario 5: Local is HLV-aware, remote is HLV-aware (ISGR peer)
647+
// ┌─────────────────────────────────────────────────────┐
648+
// │ history property: [hlvHistory] │
649+
// │ revTreeHistory prop: [revID, revTreeHistory...] │
650+
// │ │
651+
// │ ISGR peers (sendRevTreeProperty) get HLV in the │
652+
// │ history property and rev tree in a separate │
653+
// │ revTreeHistory property to keep both reconciled. │
654+
// └─────────────────────────────────────────────────────┘
655+
func (bsc *BlipSyncContext) buildRevHistory(input revHistoryInput) (history []string, revTreeHistoryProperty []string) {
656+
// Build main history: rev tree for legacy/pre-HLV, HLV for upgraded docs
657+
if !bsc.useHLV() || input.localIsLegacyRev {
658+
history = input.revTreeHistory // Scenario 1, 4
596659
} else {
597-
history = revDelta.RevisionHistory
660+
if input.hlvHistory != "" {
661+
history = append(history, input.hlvHistory) // Scenarios 2, 3, 5
662+
}
598663
}
599-
if bsc.sendRevTreeProperty() {
600-
revTreeProperty = append(revTreeProperty, revDelta.ToRevID)
601-
revTreeProperty = append(revTreeProperty, revDelta.RevisionHistory...)
664+
665+
// Append or separate the rev tree for cross-version or ISGR peer scenarios
666+
if input.remoteIsLegacyRev && !input.localIsLegacyRev {
667+
// Scenario 3: remote needs rev tree appended to history for conflict detection
668+
history = append(history, input.revID)
669+
history = append(history, input.revTreeHistory...)
670+
} else if bsc.sendRevTreeProperty() && !input.localIsLegacyRev {
671+
// Scenario 5: ISGR peer gets rev tree in a separate property
672+
revTreeHistoryProperty = append(revTreeHistoryProperty, input.revID)
673+
revTreeHistoryProperty = append(revTreeHistoryProperty, input.revTreeHistory...)
602674
}
675+
676+
return history, revTreeHistoryProperty
677+
}
678+
679+
func (bsc *BlipSyncContext) sendDelta(ctx context.Context, sender *blip.Sender, docID string, collectionIdx *int, deltaSrcRevID string, revDelta *RevisionDelta, seq SequenceID, remoteIsLegacyRev bool, resendFullRevisionFunc func() error) error {
680+
681+
// ToRevID is always a rev tree ID; use ToCV to determine if the doc has been upgraded to HLV
682+
localIsLegacyRev := revDelta.ToCV == ""
683+
684+
history, revTreeProperty := bsc.buildRevHistory(revHistoryInput{
685+
revTreeHistory: revDelta.RevisionHistory,
686+
hlvHistory: revDelta.HlvHistory,
687+
revID: revDelta.ToRevID,
688+
localIsLegacyRev: localIsLegacyRev,
689+
remoteIsLegacyRev: remoteIsLegacyRev,
690+
})
603691
properties, err := blipRevMessageProperties(history, revDelta.ToDeleted, seq, "", revTreeProperty)
604692
if err != nil {
605693
return err
@@ -755,50 +843,25 @@ func (bsc *BlipSyncContext) sendRevision(ctx context.Context, sender *blip.Sende
755843
if replacedRevID != "" {
756844
bsc.replicationStats.SendReplacementRevCount.Add(1)
757845
}
758-
var history []string
759-
if !bsc.useHLV() || localIsLegacyRev {
846+
// Compute rev tree history when any scenario needs it (see buildRevHistory for scenario docs)
847+
var revTreeHistory []string
848+
if !bsc.useHLV() || localIsLegacyRev || remoteIsLegacyRev || bsc.sendRevTreeProperty() {
760849
var err error
761-
history, err = toHistory(docRev.History, knownRevs, maxHistory)
850+
revTreeHistory, err = toHistory(docRev.History, knownRevs, maxHistory)
762851
if err != nil {
763852
err := base.RedactErrorf("Could not get rev tree history for %s %s: %w, sending a noRev to skip this revision for replication at sequence %s.", base.UD(docID), revID, err, seq)
764853
base.WarnfCtx(ctx, "%s", err)
765854
return bsc.sendNoRev(sender, docID, revID, collectionIdx, seq, err)
766855
}
767-
} else {
768-
if docRev.HlvHistory != "" {
769-
history = append(history, docRev.HlvHistory)
770-
}
771856
}
772857

773-
var revTreeHistoryProperty []string
774-
// If the remote side of the replication has this document but it's a legacy version of the document, we need to send the full rev tree history
775-
// after the hlv history so the remote side can identify iof the document is in conflict or not. We should only do
776-
// this when the local revision is not a legacy revision as if local revision is also legacy revision we will have built
777-
// the full rev tree history above to send in the history property.
778-
if remoteIsLegacyRev && !localIsLegacyRev {
779-
// append current revID and rest of rev tree after hlv history
780-
revTreeHistory, err := toHistory(docRev.History, knownRevs, maxHistory)
781-
if err != nil {
782-
err := base.RedactErrorf("Could not get rev tree history for %s %s when remote revision is a legacy rev and the local is hlv aware: %w, sending a noRev to skip this revision for replication at sequence %d.", base.UD(docID), revID, err, seq)
783-
base.WarnfCtx(ctx, "%v", err)
784-
return bsc.sendNoRev(sender, docID, revID, collectionIdx, seq, err)
785-
}
786-
history = append(history, docRev.RevID)
787-
history = append(history, revTreeHistory...)
788-
} else if bsc.sendRevTreeProperty() && !localIsLegacyRev {
789-
// If we are communicating in > 4 subprotocol versions and the client is another SGW peer, we have a revTree
790-
// property we can populate with the rev tree to keep rev tree reconciled on the replication. This property
791-
// should only be sent if the local + remote revisions are not a legacy revisions as the rev tree in this case will be
792-
// sent in history property.
793-
revTreeHistoryProperty = append(revTreeHistoryProperty, docRev.RevID) // we need current rev
794-
history, err := toHistory(docRev.History, knownRevs, maxHistory)
795-
if err != nil {
796-
err := base.RedactErrorf("Could not get rev tree history for %s %s when local and remote revision are hlv aware: %w, sending a noRev to skip this revision for replication at sequence %s.", base.UD(docID), revID, err, seq)
797-
base.WarnfCtx(ctx, "%v", err)
798-
return bsc.sendNoRev(sender, docID, revID, collectionIdx, seq, err)
799-
}
800-
revTreeHistoryProperty = append(revTreeHistoryProperty, history...)
801-
}
858+
history, revTreeHistoryProperty := bsc.buildRevHistory(revHistoryInput{
859+
revTreeHistory: revTreeHistory,
860+
hlvHistory: docRev.HlvHistory,
861+
revID: docRev.RevID,
862+
localIsLegacyRev: localIsLegacyRev,
863+
remoteIsLegacyRev: remoteIsLegacyRev,
864+
})
802865

803866
properties, err := blipRevMessageProperties(history, docRev.Deleted, seq, replacedRevID, revTreeHistoryProperty)
804867
if err != nil {

0 commit comments

Comments
 (0)