CBG-3690 don't re-read document for resync

[torcolvin]

CBG-3690 don't re-read document for resync

• one fix to create hlv if none exists from CBG-5017 Resync: allow legacy rev documents to be resynced #7890
• remove non xattr code pathway
• removed redundant args from resyncDocument and unused return param

Pre-review checklist

• Removed debug logging (fmt.Print, log.Print, ...)
• Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
• Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Integration Tests

• GSI=true,xattrs=true https://jenkins.sgwdev.com/job/SyncGatewayIntegration/171/ (unrelated flake in TestRemoveIndexesUseViewsTrueAndFalse)

[Copilot]

Pull Request Overview

This pull request optimizes the document resync operation by eliminating unnecessary document re-reads from the bucket. The key changes improve performance by passing the document data directly from DCP feed events to the resync function, and add support for creating HLV (Hybrid Logical Vector) metadata during resync operations.

Key Changes

• Modified resyncDocument to accept a *sgbucket.BucketDocument parameter instead of re-reading from the bucket
• Added HLV creation logic to the resync path for documents that don't have HLV
• Removed the non-xattr code pathway from resync operations

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
db/database.go	Updated resyncDocument signature to accept bucket document directly; added HLV update logic during resync
db/background_mgr_resync_dcp.go	Modified DCP callback to convert feed events to bucket documents and pass to resync
db/crud.go	Extended HLV update logic to handle Resync case alongside Import
db/document.go	Added bucketDocumentFromFeed and getBucketDocument helper functions; removed unused UnmarshalDocumentFromFeed
db/document_test.go	Added test for getBucketDocument function
db/database_test.go	Enhanced resync tests to verify HLV creation and metadata-only updates
base/constants.go	Added VirtualExpiry constant for fetching document expiry

[torcolvin]

When looking at this PR, evaluate this for safety to put into 4.0.2 to address CBG-5017 and obviate the need for #7890

[torcolvin]

To consider:

Is the latest set of changes better than https://github.com/couchbase/sync_gateway/pull/7892/files/eb809bf36ded144a008de3741c629a11a999d827 ? Does it add more risk to the ticket to make it not worthwhile? The idea behind the latest set of changes is to avoid having to set mouMatch and moving the update to a single point in code. This means that we move

The refactor exposes something that is tricky which is that Document.RevSeqNo is present on the Document struct but only populated when an import is occurring, since we do not want to fetch an extra xattr if needed. This means the value is often 0, so I added code to check if Document.RevSeqNo = 0 and not write the mou in this case. An error here would be a developer problem.

I thought about if the right thing to do is to add RevSeqNo to sgbucket.BucketDocument which almost fixes the problem, except for on of the on demand import fields on write. Because documentUpdateFunc takes db.Document object, we need to be able to pass rev seq no in that function. That refactor became even bigger than the existing refactor that I did to consolidate mou handling.

[torcolvin]

To consider:

Is the latest set of changes better than couchbase/sync_gateway/pull/7892/files/eb809bf36ded144a008de3741c629a11a999d827 ? Does it add more risk to the ticket to make it not worthwhile? The idea behind the latest set of changes is to avoid having to set mouMatch and moving the update to a single point in code. This means that we move

The refactor exposes something that is tricky which is that Document.RevSeqNo is present on the Document struct but only populated when an import is occurring, since we do not want to fetch an extra xattr if needed. This means the value is often 0, so I added code to check if Document.RevSeqNo = 0 and not write the mou in this case. An error here would be a developer problem.

I thought about if the right thing to do is to add RevSeqNo to sgbucket.BucketDocument which almost fixes the problem, except for on of the on demand import fields on write. Because documentUpdateFunc takes db.Document object, we need to be able to pass rev seq no in that function. That refactor became even bigger than the existing refactor that I did to consolidate mou handling.

This entire comment is obviated since it was determined to always update _mou and never _vv in 693e4b5

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.