Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBG-3725: [3.1.4 backport] Import DCP rollback unsuccessful in data migration cases #6676

Merged
merged 3 commits into from
Feb 9, 2024

Conversation

gregns1
Copy link
Contributor

@gregns1 gregns1 commented Feb 9, 2024

CBG-3725

backport of CBG-3724: Import DCP rollback unsuccessful in data migration cases

Had to remove some problematic code from dcp_receiver.go

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

torcolvin and others added 2 commits February 9, 2024 12:23
* CBG-3724 ensure import rollback works correctly

Major fix: pass a function to call cbgt.JANITOR_ROLLBACK_PINDEX when
creating a pindex. This is necessary to ensure cbgt.Mgr will actually
generate the index.

- fix the TestImportRollback test which was giving a false pass before
  because the cbgt.Mgr was getting a kick op when the nodes were
  rebalanced. In order for rollback to work, the cbgt.Dest RollbackEx
  function has to force cgbt.Mgr to actually update the PIndex. A
  cbgt.JANITOR_ROLLBACK_PINDEX op will force the recreation of a PIndex.
  This test was broken before adding code to it.
- Enhance the TestImportRollback to detect a rollback with invalid FailoverLogs.
  This simulates a situation where the checkpoints have been migrated
  from a separate bucket.

- modify TestContinuousRollback to also test with bad vbuuid FailoverEntry.
  This test actually would pass, because Sync Gateway, unlike cbgt, did
  not pass DcpStreamAddFlagStrictVBUUID. DcpStreamAddFlagStrictVBUUID
  forces a rollback in the case where StartSeqNo=0 vbuuid=invalid
  whereas without the flag it is treated like StartSeqNo=0 vbuuid=0.
  Changed the code to match cbgt behavior adding
  DcpStreamAddFlagStrictVBUUID.
- Added a separate flag in DCPClient to detect active vBuckets, which
  are the number of vBuckets that actually have a connection, and are
  not stuck in a connecting state. This differs from the existing
  activeVbuckets which is set from the time of NewDCPClient to when the
  streams end. In a continuous feed, the streams should never end.

Cleanup:

- move RollbackEx function from DCPDest to DCPCommon. DCPCommon is an
  embedded struct in DCPDest.
- panic if DCPCommon.Rollback is called. This should never happen given
  the design of cbgt.Dest, but if it did, the rollback would not work.
  This is different from the old behavior where it would not rollback
  and potentially enter a rollback loop if this function was called.
- remove maxVbNo from NewDCPDest in effort to shorten the arguments
  since the flag can be inferred from base.Bucket.
- Create some public structs to make tests easier:
	- ShardedImportDCPMetadata
	- CBGCfgIndexDefs, CBGTIndexNodeDefsKnown, CBGTIndexNodeDefsWanted, CBGTCfgPlanPIndexes

* work around MB-60564

* Change message

* Make remove raceproof and improve comment

* Reduce scope of changes for easier backport

* Address comments to make code simpler
Co-authored-by: Tor Colvin <[email protected]>
@torcolvin torcolvin enabled auto-merge (squash) February 9, 2024 14:40
@torcolvin torcolvin merged commit d3b758c into release/3.1.4 Feb 9, 2024
23 of 24 checks passed
@torcolvin torcolvin deleted the CBG-3725 branch February 9, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants