CBG-4389: extract cv from known revs and store backup rev by cv #7237
Conversation
gregns1
changed the title
gregns1
changed the title
db/blip_handler.go
Outdated
| @@ -1082,6 +1084,9 @@ func (bh *blipHandler) processRev(rq *blip.Message, stats *processRevStats) (err | |||
| if deltaSrcRevID, isDelta := revMessage.DeltaSrc(); isDelta && !revMessage.Deleted() { | |||
| if !bh.sgCanUseDeltas { | |||
| return base.HTTPErrorf(http.StatusBadRequest, "Deltas are disabled for this peer") | |||
| } else if !bh.useHLV() { | |||
There was a problem hiding this comment.
For an incoming delta that has a revTreeID as deltaSrc, we should be able to accept if when deltaSrc is the current revTreeID. That case shouldn't require any temporary revision bodies.
There was a problem hiding this comment.
Good point I think as long as the delta source rev is in the rev cache its fine, I think it makes sense for me to remove this upon reflection. If we receive a rev delta source, look in rev cache for it and fail to find it (then fail to load as no backup by rev id), the function will error anyway on line 1110.
db/blip_sync_context.go
Outdated
| if bsc.useHLV() { | ||
| msgHLV, err := extractHLVFromBlipMessage(revID) | ||
| if err != nil { | ||
| base.ErrorfCtx(ctx, "Invalid known rev format for hlv on doc: %s", docID) |
There was a problem hiding this comment.
If we can't parse this, should we fall back to sending full body instead of erroring? Or will it cause problems elsewhere?
There was a problem hiding this comment.
On a related note, does the loop in line 374 also need to handle HLV format, or does that happen elsewhere?
There was a problem hiding this comment.
Good spot! Have updated to reflect this, let me know what you think!
| @@ -2611,6 +2605,24 @@ func postWriteUpdateHLV(doc *Document, casOut uint64) *Document { | |||
| if doc.HLV.CurrentVersionCAS == expandMacroCASValueUint64 { | |||
| doc.HLV.CurrentVersionCAS = casOut | |||
| } | |||
| // backup new revision to the bucket now we have a doc assigned a CV (post macro expansion) for delta generation purposes | |||
| backupRev := db.deltaSyncEnabled() && db.deltaSyncRevMaxAgeSeconds() != 0 | |||
| if db.UseXattrs() && backupRev { | |||
There was a problem hiding this comment.
I think the UseXattrs() check is unnecessary here (we don't support HLV with non-xattrs)
* CBG-4389: extract cv from knwon revs and store backup rev by revID * update comments * fix backup revs * further tidy up * udpated to address comments and fix flaking tests * fix incorrect fetch format by cv in getCurrentVersion
CBG-4389
Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiIntegration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGateway-Integration/2862/