Active-passive to active-active domain migration support #7071
Conversation
| // we increment failover version so top level failoverVersion is updated and domain data is replicated. | ||
| failoverVersion = d.clusterMetadata.GetNextFailoverVersion( | ||
| replicationConfig.ActiveClusterName, | ||
| failoverVersion+1, |
There was a problem hiding this comment.
GetNextFailoverVersion(0) returns 0. I need it to actually increase.
| When an active-passive domain is migrated to active-active, | ||
| - The domain record will have the `ActiveClusters` field set | ||
| - The existing `ActiveClusterName` field will be left as is. | ||
| - The failover version of the domain will be incremented. Even though this is not used for task versions for active-active domains, it's incremented to indicate there was a change in replication config. |
There was a problem hiding this comment.
I'm kinda afraid that this can cause confusion to people. For active-active domain and active-passive domains, this field will have different semantics. Should we deprecate this field for active-active domain and use a different field?
There was a problem hiding this comment.
We are gonna hide it in UI & CLI response so shouldn't cause confusion unless DB is inspected. However it's not a bad idea to not use this field for active-active domains at all. That requires further changes in domain replication such as this condition. I'll leave that as TODO(active-active) and we can evaluate later
| // top level failover version is not used for task versions for active-active domains but we still increment it | ||
| // to indicate there was a change in replication config | ||
| failoverVersion = d.clusterMetadata.GetNextFailoverVersion( | ||
| d.clusterMetadata.GetCurrentClusterName(), |
There was a problem hiding this comment.
Why do we use current cluster here and active cluster above?
There was a problem hiding this comment.
Active-active domain doesn't have a current cluster (unless it's migrated) but we still need to increase the failover version due to the reason I mentioned in other comment.
What changed?
Why?
Avoid requiring creating new domains to leverage active-active mode by supporting migration
How did you test it?