Skip to content

Commit 5b41b06

Browse files
torcolvinCopilot
andauthored
CBG-4878 Set _mou from attachment migration (#7781)
Co-authored-by: Copilot <[email protected]>
1 parent 5bf2d12 commit 5b41b06

File tree

2 files changed

+27
-25
lines changed

2 files changed

+27
-25
lines changed

db/crud.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,7 +1051,7 @@ func (db *DatabaseCollectionWithUser) updateHLV(ctx context.Context, d *Document
10511051

10521052
// MigrateAttachmentMetadata will move any attachment metadata defined in sync data to global sync xattr
10531053
func (c *DatabaseCollectionWithUser) MigrateAttachmentMetadata(ctx context.Context, docID string, cas uint64, syncData *SyncData) error {
1054-
xattrs, _, err := c.dataStore.GetXattrs(ctx, docID, []string{base.GlobalXattrName})
1054+
xattrs, _, err := c.dataStore.GetXattrs(ctx, docID, []string{base.GlobalXattrName, base.VirtualXattrRevSeqNo})
10551055
if err != nil && !base.IsXattrNotFoundError(err) {
10561056
return err
10571057
}
@@ -1073,14 +1073,32 @@ func (c *DatabaseCollectionWithUser) MigrateAttachmentMetadata(ctx context.Conte
10731073
if err != nil {
10741074
return base.RedactErrorf("Failed to Marshal sync data when attempting to migrate sync data attachments to global xattr with id: %s. Error: %v", base.UD(docID), err)
10751075
}
1076+
revSeqNo, err := unmarshalRevSeqNo(xattrs[base.VirtualXattrRevSeqNo])
1077+
if err != nil {
1078+
base.InfofCtx(ctx, base.KeyCRUD, "Could not determine revSeqNo found when attempting to migrate sync data attachments to global xattr for doc %q. Assuming 0. Error: %v", base.UD(docID), err)
1079+
}
1080+
1081+
metadataOnlyUpdate := &MetadataOnlyUpdate{
1082+
HexCAS: expandMacroCASValueString, // when non-empty, this is replaced with cas macro expansion
1083+
PreviousHexCAS: syncData.Cas,
1084+
PreviousRevSeqNo: revSeqNo,
1085+
}
1086+
rawMouXattr, err := base.JSONMarshal(metadataOnlyUpdate)
1087+
if err != nil {
1088+
return base.RedactErrorf("Failed to marshal _mou when attempting to migrate sync data attachments to global xattr with id: %s. Error: %v", base.UD(docID), err)
1089+
}
10761090

10771091
// build macro expansion for sync data. This will avoid the update to xattrs causing an extra import event (i.e. sync cas will be == to doc cas)
10781092
opts := &sgbucket.MutateInOptions{}
1079-
spec := macroExpandSpec(base.SyncXattrName)
1093+
spec := append(macroExpandSpec(base.SyncXattrName), sgbucket.NewMacroExpansionSpec(XattrMouCasPath(), sgbucket.MacroCas))
10801094
opts.MacroExpansion = spec
10811095
opts.PreserveExpiry = true // if doc has expiry, we should preserve this
10821096

1083-
updatedXattr := map[string][]byte{base.SyncXattrName: rawSyncXattr, base.GlobalXattrName: globalXattr}
1097+
updatedXattr := map[string][]byte{
1098+
base.SyncXattrName: rawSyncXattr,
1099+
base.GlobalXattrName: globalXattr,
1100+
base.MouXattrName: rawMouXattr,
1101+
}
10841102
_, err = c.dataStore.UpdateXattrs(ctx, docID, 0, cas, updatedXattr, opts)
10851103
return err
10861104
}

xdcr/xdcr_test.go

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ func TestReplicateXattrs(t *testing.T) {
630630
func TestXDCRBeforeAttachmentMigration(t *testing.T) {
631631
base.SetUpTestLogging(t, base.LevelDebug, base.KeyAll)
632632

633-
srcBucket, srcDs, dstBucket, dstDs := getTwoBucketDataStores(t)
633+
srcBucket, srcDs, dstBucket, _ := getTwoBucketDataStores(t)
634634
ctx := base.TestCtx(t)
635635

636636
srcDB, srcCtx := db.CreateTestDatabase(t, base.NoCloseClone(srcBucket), db.DatabaseContextOptions{EnableXattr: true, Scopes: db.GetScopesOptions(t, srcBucket, 1)})
@@ -675,7 +675,7 @@ func TestXDCRBeforeAttachmentMigration(t *testing.T) {
675675
requireWaitForXDCRDocsWritten(t, xdcr, 2)
676676

677677
// Verify doc is present and attachment wasn't found - via REST-like call Get1xRevAndChannels
678-
b, _, _, _, _, dstPreMigrateSeq, dstPreMigrateRev, _, err := dstColl.Get1xRevAndChannels(dstCtx, docID, "", false)
678+
b, _, _, _, _, _, _, _, err := dstColl.Get1xRevAndChannels(dstCtx, docID, "", false)
679679
require.NoError(t, err)
680680
attsLocal := db.GetAttachmentsFromInlineBody(t, b)
681681
require.Len(t, attsLocal, 0)
@@ -686,28 +686,12 @@ func TestXDCRBeforeAttachmentMigration(t *testing.T) {
686686
sd := db.GetRawSyncXattr(t, srcDs, docID)
687687
require.NoError(t, srcColl.MigrateAttachmentMetadata(srcColl.AddCollectionContext(srcCtx), docID, fromCas, &sd))
688688

689-
// Wait for the migration's _globalSync xattr update to replicate to the target
690-
requireWaitForXDCRDocsWritten(t, xdcr, 3)
689+
// Since metadata migration writes _mou, the document will be processed. Wait for the migration's _globalSync xattr update to replicate to the target.
690+
requireWaitForXDCRDocsProcessed(t, xdcr, 4)
691691

692-
// check that we can fetch the attachment now
693-
// Due to CBG-4878, this writes a new revision since _vv.ver > _sync.rev.ver even though the body didn't change.
694-
// This isn't ideal and it would be better to not have this issue a new revision.
695-
//
696-
// Note: The versions remaining the same means that clients that observe the missing attachment never get it until a subsequent doc update
697-
// The benefit of doing this though, is that every other client (who has had this attachment) doesn't get a no-op document update post-migration.
698-
b, _, _, _, _, dstPostMigrateSeq, dstPostMigrateRev, _, err := dstColl.Get1xRevAndChannels(dstCtx, docID, "", false)
692+
stats, err := xdcr.Stats(ctx)
699693
require.NoError(t, err)
700-
atts := db.GetAttachmentsFromInlineBody(t, b)
701-
require.Len(t, atts, 1)
702-
assert.Contains(t, atts, attName)
703-
704-
// Also assert _globalSync.attachments_meta present on target
705-
attachmentsAfter := db.GetRawGlobalSyncAttachments(t, dstDs, docID)
706-
require.Contains(t, attachmentsAfter, attName)
707-
708-
// CBG-4878 causes these to not be equal but equality would be better.
709-
assert.NotEqual(t, dstPreMigrateSeq, dstPostMigrateSeq)
710-
assert.NotEqual(t, dstPreMigrateRev, dstPostMigrateRev)
694+
require.Equal(t, uint64(2), stats.DocsWritten)
711695
}
712696

713697
// TestVVMultiActor verifies that updates by multiple actors (updates to different clusters/buckets) are properly

0 commit comments

Comments
 (0)