Skip to content

Commit

Permalink
Bug fix: track refs by ID, instead of by path, since branches and tag…
Browse files Browse the repository at this point in the history
…s with the same name will have the same path
  • Loading branch information
fulghum committed Dec 7, 2024
1 parent 24f85b3 commit cf1edd8
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 7 deletions.
14 changes: 7 additions & 7 deletions go/libraries/doltcore/sqle/read_replica_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,25 +275,25 @@ type pullBehavior bool
const pullBehaviorFastForward pullBehavior = false
const pullBehaviorForcePull pullBehavior = true

// pullBranches pulls the remote branches named and returns the map of their hashes keyed by branch path.
// pullBranches pulls the named remote branches and tags and returns the map of their hashes keyed by ref ID.
func pullBranches(
ctx *sql.Context,
rrd ReadReplicaDatabase,
remoteRefs []doltdb.RefWithHash,
localRefs []doltdb.RefWithHash,
behavior pullBehavior,
) (map[string]doltdb.RefWithHash, error) {
localRefsByPath := make(map[string]doltdb.RefWithHash)
remoteRefsByPath := make(map[string]doltdb.RefWithHash)
localRefsById := make(map[string]doltdb.RefWithHash)
remoteRefsById := make(map[string]doltdb.RefWithHash)
remoteHashes := make([]hash.Hash, len(remoteRefs))

for i, b := range remoteRefs {
remoteRefsByPath[b.Ref.GetPath()] = b
remoteRefsById[b.Ref.String()] = b
remoteHashes[i] = b.Hash
}

for _, b := range localRefs {
localRefsByPath[b.Ref.GetPath()] = b
localRefsById[b.Ref.String()] = b
}

// XXX: Our view of which remote branches to pull and what to set the
Expand All @@ -311,7 +311,7 @@ func pullBranches(
REFS: // every successful pass through the loop below must end with `continue REFS` to get out of the retry loop
for _, remoteRef := range remoteRefs {
trackingRef := ref.NewRemoteRef(rrd.remote.Name, remoteRef.Ref.GetPath())
localRef, localRefExists := localRefsByPath[remoteRef.Ref.GetPath()]
localRef, localRefExists := localRefsById[remoteRef.Ref.String()]

// loop on optimistic lock failures
OPTIMISTIC_RETRY:
Expand Down Expand Up @@ -378,7 +378,7 @@ func pullBranches(
return nil, err
}

return remoteRefsByPath, nil
return remoteRefsById, nil
}

// expandWildcardBranchPattern evaluates |pattern| and returns a list of branch names from the source database that
Expand Down
38 changes: 38 additions & 0 deletions integration-tests/bats/replication.bats
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,44 @@ teardown() {
[[ ! "$output" =~ "feature" ]] || false
}

# Asserts that when a branch is deleted and a tag is created and they have the same name,
# the replica is able to correctly apply both changes.
@test "replication: pull branch delete and tag create with same name on read" {
# Configure repo1 to push changes on commit and create branch b1
cd repo1
dolt config --local --add sqlserver.global.dolt_replicate_to_remote remote1
dolt sql -q "call dolt_branch('b1');"

# Configure repo2 to pull changes on read and assert the b1 branch exists
cd ..
dolt clone file://./rem1 repo2
cd repo2
dolt config --local --add sqlserver.global.dolt_read_replica_remote origin
dolt config --local --add sqlserver.global.dolt_replicate_all_heads 1
run dolt sql -q "select name from dolt_branches" -r csv
[ "$status" -eq 0 ]
[ "${#lines[@]}" -eq 3 ]
[[ "$output" =~ "name" ]] || false
[[ "$output" =~ "main" ]] || false
[[ "$output" =~ "b1" ]] || false

# Delete branch b1 in repo1 and create tag b1
cd ../repo1
dolt sql -q "call dolt_branch('-D', 'b1'); call dolt_tag('b1');"

# Confirm that branch b1 is deleted and tag b1 is created in repo2
cd ../repo2
run dolt sql -q "select name from dolt_branches" -r csv
[ "$status" -eq 0 ]
[ "${#lines[@]}" -eq 2 ]
[[ "$output" =~ "main" ]] || false
run dolt sql -q "select tag_name from dolt_tags" -r csv
[ "$status" -eq 0 ]
[ "${#lines[@]}" -eq 2 ]
[[ "$output" =~ "tag_name" ]] || false
[[ "$output" =~ "b1" ]] || false
}

@test "replication: pull branch delete current branch" {
skip "broken by latest transaction changes"

Expand Down

0 comments on commit cf1edd8

Please sign in to comment.