This is an automated email from the ASF dual-hosted git repository.
zrhoffman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficcontrol.git
The following commit(s) were added to refs/heads/master by this push:
new 592780aebf Minor TM refactor to simplify statecombiner logic (#7122)
592780aebf is described below
commit 592780aebfc043c0df3e06f3c62b593b736e33b9
Author: Rawlin Peters <[email protected]>
AuthorDate: Wed Oct 12 09:23:02 2022 -0600
Minor TM refactor to simplify statecombiner logic (#7122)
This does not actually change the behavior in any way -- it just
simplifies the logic by removing a hard-coded boolean parameter.
---
traffic_monitor/manager/statecombiner.go | 77 +++++++++++++--------------
traffic_monitor/manager/statecombiner_test.go | 6 +--
2 files changed, 38 insertions(+), 45 deletions(-)
diff --git a/traffic_monitor/manager/statecombiner.go
b/traffic_monitor/manager/statecombiner.go
index 9eb45745a4..3fdf4f0bc8 100644
--- a/traffic_monitor/manager/statecombiner.go
+++ b/traffic_monitor/manager/statecombiner.go
@@ -47,7 +47,7 @@ func StartStateCombiner(events health.ThreadsafeEvents,
peerStates peer.CRStates
go func() {
overrideMap := map[tc.CacheName]bool{}
for range combineStateChan {
- combineCrStates(events, true,
peerStates.GetCRStatesPeersInfo(), localStates.Get(), combinedStates,
overrideMap, toData.Get())
+ combineCrStates(events,
peerStates.GetCRStatesPeersInfo(), localStates.Get(), combinedStates,
overrideMap, toData.Get())
}
}()
@@ -58,7 +58,6 @@ func combineCacheState(
cacheName tc.CacheName,
localCacheState tc.IsAvailable,
events health.ThreadsafeEvents,
- peerOptimistic bool,
peerCrStatesInfo peer.CRStatesPeersInfo,
combinedStates peer.CRStatesThreadsafe,
overrideMap map[tc.CacheName]bool,
@@ -77,46 +76,42 @@ func combineCacheState(
overrideCondition = "cleared; healthy locally"
overrideMap[cacheName] = false
}
- } else if peerOptimistic {
- if !peerCrStatesInfo.HasAvailablePeers() {
- if override {
- overrideCondition = "irrelevant; no peers
online"
- overrideMap[cacheName] = false
- }
- } else {
- onlineOnPeers := make([]string, 0)
- ipv4OnlineOnPeers := make([]string, 0)
- ipv6OnlineOnPeers := make([]string, 0)
-
- for peerName, peerCrStates := range
peerCrStatesInfo.GetCrStates() {
- if
peerCrStatesInfo.GetPeerAvailability(peerName) {
- if
peerCrStates.Caches[cacheName].IsAvailable {
- onlineOnPeers =
append(onlineOnPeers, peerName.String())
- }
- if
peerCrStates.Caches[cacheName].Ipv4Available {
- ipv4OnlineOnPeers =
append(ipv4OnlineOnPeers, peerName.String())
- }
- if
peerCrStates.Caches[cacheName].Ipv6Available {
- ipv6OnlineOnPeers =
append(ipv6OnlineOnPeers, peerName.String())
- }
+ } else if !peerCrStatesInfo.HasAvailablePeers() {
+ if override {
+ overrideCondition = "irrelevant; no peers online"
+ overrideMap[cacheName] = false
+ }
+ } else {
+ onlineOnPeers := make([]string, 0)
+ ipv4OnlineOnPeers := make([]string, 0)
+ ipv6OnlineOnPeers := make([]string, 0)
+
+ for peerName, peerCrStates := range
peerCrStatesInfo.GetCrStates() {
+ if peerCrStatesInfo.GetPeerAvailability(peerName) {
+ if peerCrStates.Caches[cacheName].IsAvailable {
+ onlineOnPeers = append(onlineOnPeers,
peerName.String())
+ }
+ if peerCrStates.Caches[cacheName].Ipv4Available
{
+ ipv4OnlineOnPeers =
append(ipv4OnlineOnPeers, peerName.String())
+ }
+ if peerCrStates.Caches[cacheName].Ipv6Available
{
+ ipv6OnlineOnPeers =
append(ipv6OnlineOnPeers, peerName.String())
}
}
+ }
- if len(onlineOnPeers) > 0 {
- available = true
- ipv4Available = ipv4Available ||
len(ipv4OnlineOnPeers) > 0 // optimistically accept true from local or peer
- ipv6Available = ipv6Available ||
len(ipv6OnlineOnPeers) > 0 // optimistically accept true from local or peer
+ if len(onlineOnPeers) > 0 {
+ available = true
+ ipv4Available = ipv4Available || len(ipv4OnlineOnPeers)
> 0 // optimistically accept true from local or peer
+ ipv6Available = ipv6Available || len(ipv6OnlineOnPeers)
> 0 // optimistically accept true from local or peer
- if !override {
- overrideCondition =
fmt.Sprintf("detected; healthy on (at least) %s", strings.Join(onlineOnPeers,
", "))
- overrideMap[cacheName] = true
- }
- } else {
- if override {
- overrideCondition = "irrelevant; not
online on any peers"
- overrideMap[cacheName] = false
- }
+ if !override {
+ overrideCondition = fmt.Sprintf("detected;
healthy on (at least) %s", strings.Join(onlineOnPeers, ", "))
+ overrideMap[cacheName] = true
}
+ } else if override {
+ overrideCondition = "irrelevant; not online on any
peers"
+ overrideMap[cacheName] = false
}
}
@@ -188,16 +183,16 @@ func pruneCombinedDSState(combinedStates
peer.CRStatesThreadsafe, localStates tc
// pruneCombinedCaches deletes caches in combined states which have been
removed from localStates.
func pruneCombinedCaches(combinedStates peer.CRStatesThreadsafe, localStates
tc.CRStates) {
combinedCaches := combinedStates.GetCaches()
- for cacheName, _ := range combinedCaches {
+ for cacheName := range combinedCaches {
if _, ok := localStates.Caches[cacheName]; !ok {
combinedStates.DeleteCache(cacheName)
}
}
}
-func combineCrStates(events health.ThreadsafeEvents, peerOptimistic bool,
peerCrStatesInfo peer.CRStatesPeersInfo, localStates tc.CRStates,
combinedStates peer.CRStatesThreadsafe, overrideMap map[tc.CacheName]bool,
toData todata.TOData) {
+func combineCrStates(events health.ThreadsafeEvents, peerCrStatesInfo
peer.CRStatesPeersInfo, localStates tc.CRStates, combinedStates
peer.CRStatesThreadsafe, overrideMap map[tc.CacheName]bool, toData
todata.TOData) {
for cacheName, localCacheState := range localStates.Caches { //
localStates gets pruned when servers are disabled, it's the source of truth
- combineCacheState(cacheName, localCacheState, events,
peerOptimistic, peerCrStatesInfo, combinedStates, overrideMap, toData)
+ combineCacheState(cacheName, localCacheState, events,
peerCrStatesInfo, combinedStates, overrideMap, toData)
}
for deliveryServiceName, localDeliveryService := range
localStates.DeliveryService {
@@ -208,7 +203,7 @@ func combineCrStates(events health.ThreadsafeEvents,
peerOptimistic bool, peerCr
pruneCombinedCaches(combinedStates, localStates)
}
-// CacheNameSlice is a slice of cache names, which fulfills the
`sort.Interface` interface.
+// CacheGroupNameSlice is a slice of cache names, which fulfills the
`sort.Interface` interface.
type CacheGroupNameSlice []tc.CacheGroupName
func (p CacheGroupNameSlice) Len() int { return len(p) }
diff --git a/traffic_monitor/manager/statecombiner_test.go
b/traffic_monitor/manager/statecombiner_test.go
index 4c507e3d75..217069a1c7 100644
--- a/traffic_monitor/manager/statecombiner_test.go
+++ b/traffic_monitor/manager/statecombiner_test.go
@@ -55,7 +55,6 @@ func TestCombineCacheState(t *testing.T) {
},
}
events := health.NewThreadsafeEvents(1)
- peerOptimistic := true
peerStates := peer.NewCRStatesPeersThreadsafe(1)
peerStates.SetTimeout(time.Duration(rand.Int63()))
peerResult := peer.Result{
@@ -88,7 +87,7 @@ func TestCombineCacheState(t *testing.T) {
}
for _, localCacheState := range localCacheStates {
- combineCacheState(cacheName, localCacheState, events,
peerOptimistic, peerStates.GetCRStatesPeersInfo(), combinedStates, overrideMap,
toData)
+ combineCacheState(cacheName, localCacheState, events,
peerStates.GetCRStatesPeersInfo(), combinedStates, overrideMap, toData)
if !combinedStates.Get().Caches[cacheName].IsAvailable {
t.Fatalf("cache is unavailable and should be available")
@@ -111,7 +110,6 @@ func TestCombineCacheStateCacheDown(t *testing.T) {
}
events := health.NewThreadsafeEvents(1)
- peerOptimistic := true
peerStates := peer.NewCRStatesPeersThreadsafe(1)
peerStates.SetTimeout(time.Duration(rand.Int63()))
peerResult := peer.Result{
@@ -143,7 +141,7 @@ func TestCombineCacheStateCacheDown(t *testing.T) {
cacheName: tc.CacheTypeEdge,
}
- combineCacheState(cacheName, localCacheState, events, peerOptimistic,
peerStates.GetCRStatesPeersInfo(), combinedStates, overrideMap, toData)
+ combineCacheState(cacheName, localCacheState, events,
peerStates.GetCRStatesPeersInfo(), combinedStates, overrideMap, toData)
if !combinedStates.Get().Caches[cacheName].IsAvailable {
t.Fatalf("cache is unavailable and should be available")
