chore: Refactoring/domain handler updates #7403
Conversation
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
Signed-off-by: David Porter <[email protected]>
| // as a historical backwards compatibility measure. | ||
| // Going forward, any history use-cases should rely on the FailoverHistory endpoint which | ||
| // supports all failover types and more than a handful of entries. | ||
| if !wasActiveActive && !isActiveActive { |
There was a problem hiding this comment.
behaviour change, not doing to put active/active updates in the domain-data since they will likely get too hard to read and be too large.
There was a problem hiding this comment.
In long-term, are we removing failover history from data blob for active-passive domains?
There was a problem hiding this comment.
I don't really have an opinion. Probably yes, but some system parts have taken a dependency on it, so I'm not racing to do so
| lastUpdatedTime, | ||
| response, err := d.handleFailoverRequest( | ||
| ctx, | ||
| failoverRequest.ToUpdateDomainRequest(), |
There was a problem hiding this comment.
chose to map to the UpdateDomain request struct here rather than the reverse because the FailoverDomain API is actually a subset of the UpdateDomain failover capabilities, we didn't include any graceful failover fields for it, so it has to be this way
Signed-off-by: David Porter <[email protected]>
davidporter-id-au
force-pushed
the
refactoring/domain-handler-update-iii
branch
from
0375f5d to
5628316
Compare
Signed-off-by: David Porter <[email protected]>
davidporter-id-au
changed the title
davidporter-id-au
changed the title
| FailoverType string `json:"failoverType,omitempty"` | ||
|
|
||
| // active-active domain failover | ||
| FromActiveClusters types.ActiveClusters `json:"fromActiveClusters,omitempty"` |
There was a problem hiding this comment.
Intentional removal, these are to be replaced by the FailoverHistory endpoint
| }{ | ||
| { | ||
| name: "Success case - global domain force failover via replication config", | ||
| name: "Success case - active/passive domain - global domain force failover - failing over from cluster A to cluster B", |
There was a problem hiding this comment.
rewrote the test with different data to be less confusing
| DomainName: t.DomainName, | ||
| DomainActiveClusterName: *t.DomainActiveClusterName, | ||
| DomainActiveClusterName: t.GetDomainActiveClusterName(), | ||
| ActiveClusters: FromActiveClusters(t.ActiveClusters), |
There was a problem hiding this comment.
Bug, mappers were missing
davidporter-id-au
requested review from
3vilhamster,
Shaddoll,
jakobht,
neil-xie,
sankari165 and
shijiesheng
as code owners
davidporter-id-au
requested review from
demirkayaender,
dkrotx and
taylanisikdemir
as code owners
| // as a historical backwards compatibility measure. | ||
| // Going forward, any history use-cases should rely on the FailoverHistory endpoint which | ||
| // supports all failover types and more than a handful of entries. | ||
| if !wasActiveActive && !isActiveActive { |
There was a problem hiding this comment.
In long-term, are we removing failover history from data blob for active-passive domains?
| // Any parts of the system (domain-callbacks, domain cache etc) that watch for | ||
| // failover events and may trigger some action or cache invalidation will be watching | ||
| // the domain-level failver counter for changes. Therefore, by bumping it even for | ||
| // cases where it isn't changing, we ensure all these other subprocesses will |
There was a problem hiding this comment.
I still think we should find the subprocesses depending on this and extend them to support cluster-attribute-level failover instead of increasing the domain level failover version fall all failovers.
But I'm ok with the current state.
2 participants
What changed?
This is not a perfect refactor, I changed a few behaviours while I was here since I considered them low risk:
Why?
How did you test it?
Potential risks
Release notes
Documentation Changes