diff --git a/pkg/kv/kvserver/allocator/storepool/store_pool.go b/pkg/kv/kvserver/allocator/storepool/store_pool.go index 86781269b8e9..66f181230f88 100644 --- a/pkg/kv/kvserver/allocator/storepool/store_pool.go +++ b/pkg/kv/kvserver/allocator/storepool/store_pool.go @@ -1373,19 +1373,15 @@ func (sp *StorePool) getNodeLocality(nodeID roachpb.NodeID) roachpb.Locality { return sp.getNodeLocalityWithString(nodeID).locality } -// IsCrossRegion takes in two replicas and compares the locality of them based -// on their replica node IDs. It returns (bool, error) indicating whether the -// two replicas’ nodes are in different regions and if any errors occurred -// during the lookup process. -func (sp *StorePool) IsCrossRegion( +// IsCrossRegionCrossZone takes in two replicas and compares the locality of +// them based on their replica node IDs. It returns (bool, error, bool, error) +// where the boolean values indicate whether the two replicas' nodes are in +// different regions, different zones, along with any lookup errors. +func (sp *StorePool) IsCrossRegionCrossZone( firstReplica roachpb.ReplicaDescriptor, secReplica roachpb.ReplicaDescriptor, -) (bool, error) { - isCrossRegion, err := sp.getNodeLocality(firstReplica.NodeID).IsCrossRegion( +) (bool, error, bool, error) { + return sp.getNodeLocality(firstReplica.NodeID).IsCrossRegionCrossZone( sp.getNodeLocality(secReplica.NodeID)) - if err != nil { - return false, err - } - return isCrossRegion, nil } // IsStoreReadyForRoutineReplicaTransfer returns true iff the store's node is diff --git a/pkg/kv/kvserver/metrics.go b/pkg/kv/kvserver/metrics.go index a949db49288d..2d083919049b 100644 --- a/pkg/kv/kvserver/metrics.go +++ b/pkg/kv/kvserver/metrics.go @@ -966,6 +966,29 @@ var ( Measurement: "Bytes", Unit: metric.Unit_BYTES, } + metaRangeSnapShotCrossZoneSentBytes = metric.Metadata{ + Name: "range.snapshots.cross-zone.sent-bytes", + Help: `Number of snapshot bytes sent cross zone within same region or if + region tiers are not configured. This count increases for each snapshot sent + between different zones within the same region. However, if the region tiers + are not configured, this count may also include snapshot data sent between + different regions. Ensuring consistent configuration of region and zone + tiers across nodes helps to accurately monitor the data transmitted.`, + Measurement: "Bytes", + Unit: metric.Unit_BYTES, + } + metaRangeSnapShotCrossZoneRcvdBytes = metric.Metadata{ + Name: "range.snapshots.cross-zone.rcvd-bytes", + Help: `Number of snapshot bytes received cross zone within same region or if + region tiers are not configured. This count increases for each snapshot + received between different zones within the same region. However, if the + region tiers are not configured, this count may also include snapshot data + received between different regions. Ensuring consistent configuration of + region and zone tiers across nodes helps to accurately monitor the data + transmitted.`, + Measurement: "Bytes", + Unit: metric.Unit_BYTES, + } metaRangeSnapshotSendQueueLength = metric.Metadata{ Name: "range.snapshots.send-queue", Help: "Number of snapshots queued to send", @@ -2195,6 +2218,8 @@ type StoreMetrics struct { RangeSnapshotRecvUnusable *metric.Counter RangeSnapShotCrossRegionSentBytes *metric.Counter RangeSnapShotCrossRegionRcvdBytes *metric.Counter + RangeSnapShotCrossZoneSentBytes *metric.Counter + RangeSnapShotCrossZoneRcvdBytes *metric.Counter // Range snapshot queue metrics. RangeSnapshotSendQueueLength *metric.Gauge @@ -2819,6 +2844,8 @@ func newStoreMetrics(histogramWindow time.Duration) *StoreMetrics { RangeSnapshotRecvUnusable: metric.NewCounter(metaRangeSnapshotRecvUnusable), RangeSnapShotCrossRegionSentBytes: metric.NewCounter(metaRangeSnapShotCrossRegionSentBytes), RangeSnapShotCrossRegionRcvdBytes: metric.NewCounter(metaRangeSnapShotCrossRegionRcvdBytes), + RangeSnapShotCrossZoneSentBytes: metric.NewCounter(metaRangeSnapShotCrossZoneSentBytes), + RangeSnapShotCrossZoneRcvdBytes: metric.NewCounter(metaRangeSnapShotCrossZoneRcvdBytes), RangeSnapshotSendQueueLength: metric.NewGauge(metaRangeSnapshotSendQueueLength), RangeSnapshotRecvQueueLength: metric.NewGauge(metaRangeSnapshotRecvQueueLength), RangeSnapshotSendInProgress: metric.NewGauge(metaRangeSnapshotSendInProgress), diff --git a/pkg/kv/kvserver/replica_command.go b/pkg/kv/kvserver/replica_command.go index ae77bf79fe67..4bd39652ce57 100644 --- a/pkg/kv/kvserver/replica_command.go +++ b/pkg/kv/kvserver/replica_command.go @@ -3149,9 +3149,8 @@ func (r *Replica) followerSendSnapshot( r.store.metrics.DelegateSnapshotSendBytes.Inc(inc) } r.store.metrics.RangeSnapshotSentBytes.Inc(inc) - if r.store.shouldIncrementCrossRegionSnapshotMetrics(ctx, req.CoordinatorReplica, req.RecipientReplica) { - r.store.metrics.RangeSnapShotCrossRegionSentBytes.Inc(inc) - } + r.store.updateCrossLocalitySnapshotMetrics( + ctx, req.CoordinatorReplica, req.RecipientReplica, inc, true /* isSent */) switch header.Priority { case kvserverpb.SnapshotRequest_RECOVERY: diff --git a/pkg/kv/kvserver/replica_learner_test.go b/pkg/kv/kvserver/replica_learner_test.go index 9996d9c48b55..a9e5f38b20e9 100644 --- a/pkg/kv/kvserver/replica_learner_test.go +++ b/pkg/kv/kvserver/replica_learner_test.go @@ -2188,14 +2188,11 @@ type snapshotBytesMetrics struct { rcvdBytes int64 } -// getSnapshotBytesMetrics returns metrics on the number of snapshot bytes sent -// and received by a server. tc and serverIdx specify the index of the target -// server on the TestCluster TC. The function returns the total number of -// snapshot bytes sent/received, as well as a map with granular metrics on the -// number of snapshot bytes sent and received for each type of snapshot. The -// return value is of the form (totalBytes, granularMetrics), where totalBytes -// is a `snapshotBytesMetrics` struct containing the total bytes sent/received, -// and granularMetrics is the map mentioned above. +// getSnapshotBytesMetrics retrieves the count of each snapshot metric specified +// in the metricsName associated with the target serverIdx server and returns +// the result as a map. The keys in the map correspond to the strings in input +// metricsName. The corresponding value is a `snapshotBytesMetrics` struct +// containing the total bytes sent/received of the metric. func getSnapshotBytesMetrics( t *testing.T, tc *testcluster.TestCluster, serverIdx int, metricsName []string, ) map[string]snapshotBytesMetrics { @@ -2216,13 +2213,9 @@ func getSnapshotBytesMetrics( return metrics } -// getSnapshotMetricsDiff returns the delta between snapshot byte metrics -// recorded at different times. Metrics can be recorded using the -// getSnapshotBytesMetrics helper function, and the delta is returned in the -// form (totalBytes, granularMetrics). totalBytes is a -// snapshotBytesMetrics struct containing the difference in total bytes -// sent/received, and granularMetrics is the map of snapshotBytesMetrics structs -// containing deltas for each type of snapshot. +// getSnapshotMetricsDiff returns the difference between the values of +// corresponding snapshot metrics in two maps. Assumption: beforeMap and +// afterMap contain the same set of keys. func getSnapshotMetricsDiff( beforeMap map[string]snapshotBytesMetrics, afterMap map[string]snapshotBytesMetrics, ) map[string]snapshotBytesMetrics { @@ -2296,12 +2289,12 @@ func getExpectedSnapshotSizeBytes( } // This test verifies the accuracy of snapshot metrics - -// `range.snapshots.[rebalancing|cross-region].rcvd-bytes` and -// `range.snapshots.[rebalancing|cross-region].sent-bytes`. It involves adding -// two new replicas on different nodes within the cluster, resulting in two -// learner snapshots sent cross region. The test then compares the metrics prior -// to and after sending the snapshot to verify the accuracy. -func TestRebalancingAndCrossRegionSnapshotMetrics(t *testing.T) { +// `range.snapshots.[rebalancing|cross-region|cross-zone].rcvd-bytes` and +// `range.snapshots.[rebalancing|cross-region|cross-zone].sent-bytes`. It +// involves adding two new replicas on different nodes within the cluster, +// resulting in two learner snapshots sent across. The test then compares the +// metrics prior to and after sending the snapshot to verify the accuracy. +func TestRebalancingAndCrossRegionZoneSnapshotMetrics(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) @@ -2325,25 +2318,22 @@ func TestRebalancingAndCrossRegionSnapshotMetrics(t *testing.T) { } // The initial setup ensures the correct configuration for three nodes (with - // different localities), single-range. Note that server[2] is configured - // without the inclusion of a "region" tier key. + // different localities), single-range. const numNodes = 3 serverArgs := make(map[int]base.TestServerArgs) + + // The servers localities are configured so that the first snapshot sent from + // server0 to server1 is cross-region. The second snapshot sent from server0 + // to server2 is cross-zone within same region. + serverLocality := [numNodes]roachpb.Locality{ + {Tiers: []roachpb.Tier{{Key: "region", Value: "us-east"}, {Key: "az", Value: "us-east-1"}}}, + {Tiers: []roachpb.Tier{{Key: "region", Value: "us-west"}, {Key: "az", Value: "us-west-1"}}}, + {Tiers: []roachpb.Tier{{Key: "region", Value: "us-east"}, {Key: "az", Value: "us-east-2"}}}, + } for i := 0; i < numNodes; i++ { - if i == 2 { - serverArgs[i] = base.TestServerArgs{ - Locality: roachpb.Locality{ - Tiers: []roachpb.Tier{{Key: "zone", Value: fmt.Sprintf("us-east-%va", i)}}, - }, - Knobs: knobs, - } - } else { - serverArgs[i] = base.TestServerArgs{ - Locality: roachpb.Locality{ - Tiers: []roachpb.Tier{{Key: "region", Value: fmt.Sprintf("us-east-%v", i)}}, - }, - Knobs: knobs, - } + serverArgs[i] = base.TestServerArgs{ + Locality: serverLocality[i], + Knobs: knobs, } } @@ -2393,16 +2383,16 @@ func TestRebalancingAndCrossRegionSnapshotMetrics(t *testing.T) { return snapshotLength } - metrics := []string{".rebalancing", ".recovery", ".unknown", ".cross-region", ""} + metrics := []string{".rebalancing", ".recovery", ".unknown", ".cross-region", ".cross-zone", ""} // Record the snapshot metrics before anything has been sent / received. senderBefore := getSnapshotBytesMetrics(t, tc, 0 /* serverIdx */, metrics) firstReceiverBefore := getSnapshotBytesMetrics(t, tc, 1 /* serverIdx */, metrics) secReceiverBefore := getSnapshotBytesMetrics(t, tc, 2 /* serverIdx */, metrics) - // The first replica is added as a non-voter to help avoid issues in stress - // testing. A possible explanation is - if the first replica was added as a - // non-voter, it can be stuck in a state to receive the snapshot. This can - // cause failure to reach quorum during the second snapshot transfer. + // The first replica is added as a non-voter to help avoid failure in stress + // testing. A possible explanation in the failure is - if the first replica + // was added as a voter, it can be stuck in a state to receive the snapshot. + // This can cause failure to reach quorum during the second snapshot transfer. firstSnapshotLength := sendSnapshotToServer(1, tc.AddNonVoters) secSnapshotLength := sendSnapshotToServer(2, tc.AddVoters) totalSnapshotLength := firstSnapshotLength + secSnapshotLength @@ -2418,10 +2408,13 @@ func TestRebalancingAndCrossRegionSnapshotMetrics(t *testing.T) { ".rebalancing": {sentBytes: totalSnapshotLength, rcvdBytes: 0}, ".recovery": {sentBytes: 0, rcvdBytes: 0}, ".unknown": {sentBytes: 0, rcvdBytes: 0}, - // Assert that the cross-region metrics should remain unchanged (since - // server[2]'s locality does not include a "region" tier key). + // The first snapshot was sent from server0 to server1, so it is + // cross-region. ".cross-region": {sentBytes: firstSnapshotLength, rcvdBytes: 0}, - "": {sentBytes: totalSnapshotLength, rcvdBytes: 0}, + // The second snapshot was sent from server0 to server2, so it is + // cross-zone within same region. + ".cross-zone": {sentBytes: secSnapshotLength, rcvdBytes: 0}, + "": {sentBytes: totalSnapshotLength, rcvdBytes: 0}, } require.Equal(t, senderExpected, senderDelta) }) @@ -2432,10 +2425,13 @@ func TestRebalancingAndCrossRegionSnapshotMetrics(t *testing.T) { firstReceiverMetricsAfter := getSnapshotBytesMetrics(t, tc, 1 /* serverIdx */, metrics) firstReceiverDelta := getSnapshotMetricsDiff(firstReceiverBefore, firstReceiverMetricsAfter) firstReceiverExpected := map[string]snapshotBytesMetrics{ - ".rebalancing": {sentBytes: 0, rcvdBytes: firstSnapshotLength}, - ".recovery": {sentBytes: 0, rcvdBytes: 0}, - ".unknown": {sentBytes: 0, rcvdBytes: 0}, + ".rebalancing": {sentBytes: 0, rcvdBytes: firstSnapshotLength}, + ".recovery": {sentBytes: 0, rcvdBytes: 0}, + ".unknown": {sentBytes: 0, rcvdBytes: 0}, + // The first snapshot was sent from server0 to server1, so it is + // cross-region. ".cross-region": {sentBytes: 0, rcvdBytes: firstSnapshotLength}, + ".cross-zone": {sentBytes: 0, rcvdBytes: 0}, "": {sentBytes: 0, rcvdBytes: firstSnapshotLength}, } require.Equal(t, firstReceiverExpected, firstReceiverDelta) @@ -2450,9 +2446,10 @@ func TestRebalancingAndCrossRegionSnapshotMetrics(t *testing.T) { ".rebalancing": {sentBytes: 0, rcvdBytes: secSnapshotLength}, ".recovery": {sentBytes: 0, rcvdBytes: 0}, ".unknown": {sentBytes: 0, rcvdBytes: 0}, - // Assert that the cross-region metrics should remain unchanged (since - // server[2]'s locality does not include a "region" tier key). + // The second snapshot was sent from server0 to server2, so it is + // cross-zone within same region. ".cross-region": {sentBytes: 0, rcvdBytes: 0}, + ".cross-zone": {sentBytes: 0, rcvdBytes: secSnapshotLength}, "": {sentBytes: 0, rcvdBytes: secSnapshotLength}, } require.Equal(t, secReceiverExpected, secReceiverDelta) diff --git a/pkg/kv/kvserver/store_snapshot.go b/pkg/kv/kvserver/store_snapshot.go index 746f4711fbee..095a7d437c14 100644 --- a/pkg/kv/kvserver/store_snapshot.go +++ b/pkg/kv/kvserver/store_snapshot.go @@ -972,17 +972,60 @@ func (s *Store) checkSnapshotOverlapLocked( return nil } -// shouldIncrementCrossRegionSnapshotMetrics returns true if the two replicas -// given are cross-region, and false otherwise. -func (s *Store) shouldIncrementCrossRegionSnapshotMetrics( +// shouldIncrementCrossLocalitySnapshotMetrics returns (bool, bool) - indicating +// if the two given replicas are cross-region and cross-zone respectively. +func (s *Store) shouldIncrementCrossLocalitySnapshotMetrics( ctx context.Context, firstReplica roachpb.ReplicaDescriptor, secReplica roachpb.ReplicaDescriptor, -) bool { - isCrossRegion, err := s.cfg.StorePool.IsCrossRegion(firstReplica, secReplica) - if err != nil { - log.VEventf(ctx, 2, "unable to determine if snapshot is cross region %v", err) - return false +) (bool, bool) { + isCrossRegion, regionErr, isCrossZone, zoneErr := s.cfg.StorePool.IsCrossRegionCrossZone( + firstReplica, secReplica) + if regionErr != nil { + log.VEventf(ctx, 2, "unable to determine if snapshot is cross region %v", regionErr) + } + if zoneErr != nil { + log.VEventf(ctx, 2, "unable to determine if snapshot is cross zone %v", zoneErr) + } + return isCrossRegion, isCrossZone +} + +// updateCrossLocalitySnapshotMetrics updates the snapshot metrics in a more +// meaningful way. Cross-region metrics monitor activities across different +// regions. Cross-zone metrics monitor any cross-zone activities within the same +// region or if the region tiers are not configured. +func (s *Store) updateCrossLocalitySnapshotMetrics( + ctx context.Context, + firstReplica roachpb.ReplicaDescriptor, + secReplica roachpb.ReplicaDescriptor, + inc int64, + isSent bool, +) { + isCrossRegion, isCrossZone := s.shouldIncrementCrossLocalitySnapshotMetrics(ctx, firstReplica, secReplica) + if isSent { + if isCrossRegion { + if !isCrossZone { + log.VEventf(ctx, 2, "unexpected: cross region but same zone") + } else { + s.metrics.RangeSnapShotCrossRegionSentBytes.Inc(inc) + } + } else { + if isCrossZone { + s.metrics.RangeSnapShotCrossZoneSentBytes.Inc(inc) + } + } + } else { + // isReceived + if isCrossRegion { + if !isCrossZone { + log.VEventf(ctx, 2, "unexpected: cross region but same zone") + } else { + s.metrics.RangeSnapShotCrossRegionRcvdBytes.Inc(inc) + } + } else { + if isCrossZone { + s.metrics.RangeSnapShotCrossZoneRcvdBytes.Inc(inc) + } + } } - return isCrossRegion } // receiveSnapshot receives an incoming snapshot via a pre-opened GRPC stream. @@ -1101,11 +1144,8 @@ func (s *Store) receiveSnapshot( recordBytesReceived := func(inc int64) { s.metrics.RangeSnapshotRcvdBytes.Inc(inc) - - if s.shouldIncrementCrossRegionSnapshotMetrics( - ctx, header.RaftMessageRequest.FromReplica, header.RaftMessageRequest.ToReplica) { - s.metrics.RangeSnapShotCrossRegionRcvdBytes.Inc(inc) - } + s.updateCrossLocalitySnapshotMetrics( + ctx, header.RaftMessageRequest.FromReplica, header.RaftMessageRequest.ToReplica, inc, false /* isSent */) switch header.Priority { case kvserverpb.SnapshotRequest_RECOVERY: diff --git a/pkg/roachpb/metadata.go b/pkg/roachpb/metadata.go index 3b482f9420b6..ee56469552e1 100644 --- a/pkg/roachpb/metadata.go +++ b/pkg/roachpb/metadata.go @@ -642,20 +642,83 @@ func (l Locality) Matches(filter Locality) (bool, Tier) { return true, Tier{} } -// IsCrossRegion checks if both this and passed locality has a tier with "region" -// as the key. If either locality does not have a region tier, it returns -// (false, error). Otherwise, it compares their region values and returns (true, -// nil) if they are different, and (false, nil) otherwise. -func (l Locality) IsCrossRegion(other Locality) (bool, error) { - // It is unfortunate that the "region" tier key is hardcoded here. Ideally, we - // would prefer a more robust way to determine node locality regions. - region, hasRegion := l.Find("region") - otherRegion, hasRegionInOther := other.Find("region") - - if hasRegion && hasRegionInOther { - return region != otherRegion, nil - } - return false, errors.Errorf("locality must have a region tier key for cross-region comparison") +// getFirstRegionFirstZone iterates through the locality tiers and returns +// multiple values containing: +// 1. The value of the first encountered "region" tier. +// 2. A boolean indicating whether the "region" tier key was found. +// 3,4. The key and the value of the first encountered "zone" tier. +// 5. A boolean indicating whether the "zone" tier key was found. +func (l Locality) getFirstRegionFirstZone() ( + firstRegionValue string, + hasRegion bool, + firstZoneKey string, + firstZoneValue string, + hasZone bool, +) { + for _, tier := range l.Tiers { + if hasRegion && hasZone { + break + } + switch tier.Key { + case "region": + if !hasRegion { + firstRegionValue = tier.Value + hasRegion = true + } + case "zone", "availability-zone", "az": + if !hasZone { + firstZoneKey, firstZoneValue = tier.Key, tier.Value + hasZone = true + } + } + } + return firstRegionValue, hasRegion, firstZoneKey, firstZoneValue, hasZone +} + +// IsCrossRegionCrossZone returns multiple values containing: +// 1. A boolean value indicating if this and the provided locality are +// cross-region. +// 2. Error indicating if either locality does not have a "region" tier key. +// 3. A boolean value indicating if this and the provided locality are +// cross-zone. +// 4. Error indicating if either locality does not have a "zone" tier key or if +// the first "zone" tier keys used by two localities are different. +// +// Limitation: +// - It is unfortunate that the tier key is hardcoded here. Ideally, we would +// prefer a more robust way to look up node locality regions and zones. +// - Although it is technically possible for users to use “az”, “zone”, +// “availability-zone” as tier keys within a single locality, it can cause +// confusion when choosing the zone tier values for cross-zone comparison. In +// such cases, we would want to return an error. Ideally, both localities would +// be checked thoroughly for duplicate zone tier keys and key mismatches. +// However, due to frequent invocation of this function, we prefer to terminate +// the check after examining the first encountered zone tier key-value pairs. +// +// Note: it is intentional here to perform multiple locality tiers comparison in +// a single function to avoid overhead. If you are adding additional locality +// tiers comparisons, it is recommended to handle them within one tier list +// iteration. +func (l Locality) IsCrossRegionCrossZone( + other Locality, +) (isCrossRegion bool, regionErr error, isCrossZone bool, zoneErr error) { + firstRegionValue, hasRegion, firstZoneKey, firstZone, hasZone := l.getFirstRegionFirstZone() + firstRegionValueOther, hasRegionOther, firstZoneKeyOther, firstZoneOther, hasZoneOther := other.getFirstRegionFirstZone() + + isCrossRegion = firstRegionValue != firstRegionValueOther + isCrossZone = firstZone != firstZoneOther + + if !hasRegion || !hasRegionOther { + isCrossRegion = false + regionErr = errors.Errorf("localities must have a valid region tier key for cross-region comparison") + } + + if (!hasZone || !hasZoneOther) || (firstZoneKey != firstZoneKeyOther) { + isCrossZone = false + zoneErr = errors.Errorf("localities must have a valid zone tier key for cross-zone comparison") + } + + return isCrossRegion, regionErr, isCrossZone, zoneErr } // SharedPrefix returns the number of this locality's tiers which match those of diff --git a/pkg/roachpb/metadata_test.go b/pkg/roachpb/metadata_test.go index a23a191fdcad..de8940fbfaed 100644 --- a/pkg/roachpb/metadata_test.go +++ b/pkg/roachpb/metadata_test.go @@ -221,6 +221,142 @@ func TestLocalityMatches(t *testing.T) { } } +func TestLocalityIsCrossRegionCrossZone(t *testing.T) { + regionErrStr := "localities must have a valid region tier key for cross-region comparison" + zoneErrStr := "localities must have a valid zone tier key for cross-zone comparison" + + firstRegionStr := "us-east" + secRegionStr := "us-west" + firstZoneStr := "us-east-1" + secZoneStr := "us-west-1" + + makeLocalityStr := func(region string, zone string) string { + result := "" + intermediateStr := "" + if region != "" { + result = result + fmt.Sprintf("region=%s", region) + intermediateStr = "," + } + if zone != "" { + result = result + intermediateStr + fmt.Sprintf("zone=%s", zone) + } + if result == "" { + // empty locality is not allowed. + result = "invalid-key=invalid" + } + fmt.Println(result) + return result + } + + for _, tc := range []struct { + l string + other string + isCrossRegion bool + isCrossZone bool + crossRegionErr string + crossZoneErr string + }{ + // -------- Part 1: check for different zone tier alternatives -------- + // Valid tier keys, same regions and same zones. + {l: "region=us-west,zone=us-west-1", other: "region=us-west,zone=us-west-1", + isCrossRegion: false, isCrossZone: false, crossRegionErr: "", crossZoneErr: ""}, + // Valid tier keys, different regions and different zones. + {l: "region=us-west,zone=us-west-1", other: "region=us-east,zone=us-west-2", + isCrossRegion: true, isCrossZone: true, crossRegionErr: "", crossZoneErr: ""}, + // Valid tier keys, different regions and different zones. + {l: "region=us-west,availability-zone=us-west-1", other: "region=us-east,availability-zone=us-east-1", + isCrossRegion: true, isCrossZone: true, crossRegionErr: "", crossZoneErr: ""}, + // Valid tier keys, same regions and different zones. + {l: "region=us-west,az=us-west-1", other: "region=us-west,other-keys=us,az=us-east-1", + isCrossRegion: false, isCrossZone: true, crossRegionErr: "", crossZoneErr: ""}, + // Invalid zone tier key and different regions. + {l: "region=us-west,availability-zone=us-west-1", other: "region=us-east,zone=us-east-1", + isCrossRegion: true, isCrossZone: false, crossRegionErr: "", crossZoneErr: zoneErrStr}, + // Valid zone tier key (edge case), different zones and regions. + {l: "region=us-west,zone=us-west-1", other: "region=us-east,zone=us-west-2,az=us-west-1", + isCrossRegion: true, isCrossZone: true, crossRegionErr: "", crossZoneErr: ""}, + // Missing zone tier key and different regions. + {l: "region=us-west,zone=us-west-1", other: "region=us-east", + isCrossRegion: true, isCrossZone: false, crossRegionErr: "", crossZoneErr: zoneErrStr}, + // Different region and different zones with non-unique & invalid zone tier key. + {l: "region=us-west,zone=us-west-1,az=us-west-2", other: "az=us-west-1,region=us-west,zone=us-west-1", + isCrossRegion: false, isCrossZone: false, crossRegionErr: "", crossZoneErr: zoneErrStr}, + // Different regions and different zones with non-unique & valid zone tier key. + {l: "region=us-west,az=us-west-2,zone=us-west-1", other: "region=us-west,az=us-west-1", + isCrossRegion: false, isCrossZone: true, crossRegionErr: "", crossZoneErr: ""}, + // Invalid region tier key and different zones. + {l: "country=us,zone=us-west-1", other: "country=us,zone=us-west-2", + isCrossRegion: false, isCrossZone: true, crossRegionErr: regionErrStr, crossZoneErr: ""}, + // Missing region tier key and different zones. + {l: "az=us-west-1", other: "region=us-east,az=us-west-2", + isCrossRegion: false, isCrossZone: true, crossRegionErr: regionErrStr, crossZoneErr: ""}, + // Invalid region and zone tier key. + {l: "invalid-key=us-west,zone=us-west-1", other: "region=us-east,invalid-key=us-west-1", + isCrossRegion: false, isCrossZone: false, crossRegionErr: regionErrStr, crossZoneErr: zoneErrStr}, + // Invalid region and zone tier key. + {l: "country=us,dc=us-west-2", other: "country=us,dc=us-west-2", + isCrossRegion: false, isCrossZone: false, crossRegionErr: regionErrStr, crossZoneErr: zoneErrStr}, + // -------- Part 2: single region, single zone -------- + // One: (both) Two: (region) + {l: makeLocalityStr(firstRegionStr, firstZoneStr), other: makeLocalityStr(secRegionStr, ""), + isCrossRegion: true, isCrossZone: false, crossRegionErr: "", crossZoneErr: zoneErrStr}, + // One: (both) Two: (zone) + {l: makeLocalityStr(firstRegionStr, firstZoneStr), other: makeLocalityStr("", secZoneStr), + isCrossRegion: false, isCrossZone: true, crossRegionErr: regionErrStr, crossZoneErr: ""}, + // One: (region) Two: (region) + {l: makeLocalityStr(firstRegionStr, ""), other: makeLocalityStr(secRegionStr, ""), + isCrossRegion: true, isCrossZone: false, crossRegionErr: "", crossZoneErr: zoneErrStr}, + // One: (zone) Two: (zone) + {l: makeLocalityStr("", firstZoneStr), other: makeLocalityStr("", secZoneStr), + isCrossRegion: false, isCrossZone: true, crossRegionErr: regionErrStr, crossZoneErr: ""}, + // One: (region) Two: (zone) + {l: makeLocalityStr(firstRegionStr, ""), other: makeLocalityStr("", secZoneStr), + isCrossRegion: false, isCrossZone: false, crossRegionErr: regionErrStr, crossZoneErr: zoneErrStr}, + // One: (both) Two: (both) + {l: makeLocalityStr(firstRegionStr, firstZoneStr), other: makeLocalityStr(secRegionStr, secZoneStr), + isCrossRegion: true, isCrossZone: true, crossRegionErr: "", crossZoneErr: ""}, + // One: (none) Two: (none) + {l: makeLocalityStr("", ""), other: makeLocalityStr("", ""), + isCrossRegion: false, isCrossZone: false, crossRegionErr: regionErrStr, crossZoneErr: zoneErrStr}, + // One: (region) Two: (none) + {l: makeLocalityStr(firstRegionStr, ""), other: makeLocalityStr("", ""), + isCrossRegion: false, isCrossZone: false, crossRegionErr: regionErrStr, crossZoneErr: zoneErrStr}, + // One: (zone) Two: (none) + {l: makeLocalityStr("", firstZoneStr), other: makeLocalityStr("", ""), + isCrossRegion: false, isCrossZone: false, crossRegionErr: regionErrStr, crossZoneErr: zoneErrStr}, + // One: (both) Two: (none) + {l: makeLocalityStr(firstRegionStr, firstZoneStr), other: makeLocalityStr("", ""), + isCrossRegion: false, isCrossZone: false, crossRegionErr: regionErrStr, crossZoneErr: zoneErrStr}, + } { + t.Run(fmt.Sprintf("%s-crosslocality-%s", tc.l, tc.other), func(t *testing.T) { + var l Locality + var other Locality + require.NoError(t, l.Set(tc.l)) + require.NoError(t, other.Set(tc.other)) + type localities struct { + isCrossRegion bool + isCrossZone bool + crossRegionErr string + crossZoneErr string + } + isCrossRegion, crossRegionErr, isCrossZone, crossZoneErr := l.IsCrossRegionCrossZone(other) + crossRegionErrStr := "" + if crossRegionErr != nil { + crossRegionErrStr = crossRegionErr.Error() + } + crossZoneErrStr := "" + if crossZoneErr != nil { + crossZoneErrStr = crossZoneErr.Error() + } + actual := localities{isCrossRegion, isCrossZone, + crossRegionErrStr, crossZoneErrStr} + expected := localities{tc.isCrossRegion, tc.isCrossZone, + tc.crossRegionErr, tc.crossZoneErr} + require.Equal(t, expected, actual) + }) + } +} + func TestLocalitySharedPrefix(t *testing.T) { for _, tc := range []struct { a string