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

Multi-actor/Conflict tests and BTC pull replication conflict resolution #7286

Open
wants to merge 17 commits into
base: release/anemone
Choose a base branch
from

Conversation

bbrks
Copy link
Member

@bbrks bbrks commented Jan 10, 2025

Enable multi-actor and conflict tests and allow BlipTesterClient to resolve conflicts and push up as new version.

Spun off two failing subtests into new ticket (CBG-4458) to get other improvements into release/anemone

Base automatically changed from improve_blip_tester_client_locking to release/anemone January 13, 2025 09:37
@bbrks bbrks force-pushed the btc_conflict_resolution branch from 2234e45 to af9c239 Compare January 13, 2025 09:38
Copy link
Collaborator

@torcolvin torcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot this was a WIP, please feel free to disregard comments if irrelevant.

db/hybrid_logical_vector.go Outdated Show resolved Hide resolved
rest/utilities_testing_blip_client.go Outdated Show resolved Hide resolved
rest/utilities_testing_blip_client.go Show resolved Hide resolved
@bbrks bbrks force-pushed the btc_conflict_resolution branch from 56b325c to cfa0bfb Compare January 13, 2025 18:11
@bbrks bbrks marked this pull request as ready for review January 13, 2025 18:57
rest/utilities_testing_blip_client.go Outdated Show resolved Hide resolved
rest/utilities_testing_blip_client.go Outdated Show resolved Hide resolved
return DocMetadataFromDocument(doc), doc.Body(ctx)
var body db.Body
if !doc.IsDeleted() {
body = doc.Body(ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will be nil when this occurs, are we intentionally returning empty body rather than nil?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some commentary on this change in the CouchbaseLiteMockPeer implementation - since for a tombstone body, there were variations of []byte({}) vs nil vs []byte{}.

I don't think it really matters as long as all peers treat it the same. It was required sincd waitForConvergingVersion is also comparing doc bodies.

// GetDocument returns the latest version of a document. The test will fail the document does not exist.
func (p *CouchbaseLiteMockPeer) GetDocument(dsName sgbucket.DataStoreName, docID string) (DocMetadata, db.Body) {
	bodyBytes, meta := p.getLatestDocVersion(dsName, docID)
	require.NotNil(p.TB(), meta, "docID:%s not found on %s", docID, p)
	var body db.Body
	// it's easier if all clients can return consistent bodies for tombstones
	// lets just settle on nil, since we still need special handling anyway for `` vs `{}` so unmarshal doesn't barf

Comment on lines +430 to +464
func FromHistoryForHLV(history string) (*HybridLogicalVector, error) {
hlv := NewHybridLogicalVector()
// split the history string into PV and MV
versionSets := strings.Split(history, ";")
switch len(versionSets) {
case 0:
// no versions present
return hlv, nil
case 2:
// MV
mvs := strings.Split(versionSets[1], ",")
for _, mv := range mvs {
v, err := ParseVersion(mv)
if err != nil {
return nil, err
}
hlv.MergeVersions[v.SourceID] = v.Value
}
fallthrough
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was for pre-beta implementation and needs removing/changing

@bbrks bbrks self-assigned this Jan 21, 2025
@bbrks bbrks force-pushed the btc_conflict_resolution branch from 854ce7a to f7014ad Compare January 21, 2025 18:27
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