Skip to content

Commit

Permalink
[BUGFIX] DiffRemoteAndLocalState incorrectly sync changes with remote.
Browse files Browse the repository at this point in the history
When comparing FederationState between local and remote states, the update was comparing the lastIndex, but this index on remote could be lower than max index retrieved and stored from remote, to be exact, I found it could be zero forever in some case (probably a race condition, but was not able to pinpoint it).

This fix is not fixing the initial root cause: the ModifiedIndex is set to 0 in remote DC, but it is a useful workaround as it fixes the convergence if the remote server is not behaving as it should.

In that case, it would break sync of updates and the remote object would be never synced.

To be sure we don't forget anything, I also added a test over `isSame()` to be sure the objects are identical.

This was the reason for the test `TestReplication_FederationStates()` to fail randomly. This is also probably the cause for some of the integration tests to fail randomly as well.

With this fix, the command:
```
i=0; while /usr/local/bin/go test -timeout 30s github.com/hashicorp/consul/agent/consul -run '^(TestReplication_FederationStates)$'; do go clean -testcache; i=$((i + 1)); printf "$i "; done
```
That used to break on my machine in less than 20 runs is now running 150+ times without any issue.

Might also fix hashicorp#7575
  • Loading branch information
pierresouchay committed Apr 8, 2020
1 parent 74bd138 commit 437dee2
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions agent/consul/federation_state_replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (r *FederationStateReplicator) DiffRemoteAndLocalState(localRaw interface{}
for localIdx, remoteIdx = 0, 0; localIdx < len(local) && remoteIdx < len(remote); {
if local[localIdx].Datacenter == remote[remoteIdx].Datacenter {
// fedState is in both the local and remote state - need to check raft indices
if remote[remoteIdx].ModifyIndex > lastRemoteIndex {
if (remote[remoteIdx].ModifyIndex > lastRemoteIndex) || !remote[remoteIdx].IsSame(local[localIdx]) {
updates = append(updates, remote[remoteIdx])
}
// increment both indices when equal
Expand Down Expand Up @@ -171,7 +171,10 @@ func (r *FederationStateReplicator) PerformUpdates(ctx context.Context, updatesR
state2 := &dup

// Keep track of the raft modify index at the primary
state2.PrimaryModifyIndex = state.ModifyIndex
if state.ModifyIndex != 0 {
// For some reason, ModifyIndex can be zero from remote
state2.PrimaryModifyIndex = state.ModifyIndex
}

req := structs.FederationStateRequest{
Op: structs.FederationStateUpsert,
Expand Down

0 comments on commit 437dee2

Please sign in to comment.