zrhoffman commented on code in PR #7302:
URL: https://github.com/apache/trafficcontrol/pull/7302#discussion_r1097968267
##########
traffic_monitor/cache/astats.go:
##########
@@ -116,8 +119,22 @@ func astatsParse(cacheName string, rdr io.Reader, pollCTX
interface{}) (Statisti
astats.Ats["system.proc.loadavg"] = astats.System.ProcLoadavg
astats.Ats["system.proc.net.dev"] = astats.System.ProcNetDev
+ via := ctx.HTTPHeader.Get(rfc.Via)
+ if via != "" {
+ result := regexp.MustCompile(`
([^.]+)`).FindStringSubmatch(via)
Review Comment:
Naming this variable `result` makes it unclear what its purpose is or what
it's used for. Is there a more descriptive name to give it?
##########
traffic_monitor/cache/astats.go:
##########
@@ -116,8 +119,22 @@ func astatsParse(cacheName string, rdr io.Reader, pollCTX
interface{}) (Statisti
astats.Ats["system.proc.loadavg"] = astats.System.ProcLoadavg
astats.Ats["system.proc.net.dev"] = astats.System.ProcNetDev
+ via := ctx.HTTPHeader.Get(rfc.Via)
+ if via != "" {
+ result := regexp.MustCompile(`
([^.]+)`).FindStringSubmatch(via)
+ if len(result) > 0 {
+ astats.Ats[rfc.Via] = result[1]
+ }
+ }
return stats, astats.Ats, nil
} else if ctype == "text/csv" {
+ via := ctx.HTTPHeader.Get(rfc.Via)
+ if via != "" {
+ result := regexp.MustCompile(`
([^.]+)`).FindStringSubmatch(via)
Review Comment:
Same question here. Can `result` be renamed to something clearer?
##########
traffic_monitor/datareq/crstate.go:
##########
@@ -84,10 +88,58 @@ func filterDirectlyPolledCaches(crstates tc.CRStates)
tc.CRStates {
return filtered
}
-func srvTRStateSelf(localStates peer.CRStatesThreadsafe, directlyPolledOnly
bool) ([]byte, error) {
+func srvTRStateSelf(localStates peer.CRStatesThreadsafe, directlyPolledOnly
bool, toData todata.TODataThreadsafe) ([]byte, error) {
if !directlyPolledOnly {
- return tc.CRStatesMarshall(localStates.Get())
+ localStatesC := updateStatusAnycast(localStates, toData)
+ return tc.CRStatesMarshall(localStatesC)
}
- unfiltered := localStates.Get()
+ unfiltered := updateStatusAnycast(localStates, toData)
return tc.CRStatesMarshall(filterDirectlyPolledCaches(unfiltered))
}
+
+func updateStatusAnycast(localStates peer.CRStatesThreadsafe, toData
todata.TODataThreadsafe) tc.CRStates {
+ localStatesC := localStates.Get()
+ toDataC := toData.Get()
+
+ for cache, _ := range localStatesC.Caches {
+ if _, ok := toDataC.SameIpServers[cache]; ok {
+ // all server partners must be available if they are in
reported state
+ allAvailableV4 := true
+ allAvailableV6 := true
+ allIsAvailable := true
+ for partner, _ := range toDataC.SameIpServers[cache] {
+ if partnerState, ok :=
localStatesC.Caches[partner]; ok {
+ // a partner host is reported but is
marked down for too high traffic or load
+ // this host also needs to be marked
down to divert all traffic for their
+ // common anycast ip
+ if
strings.Contains(partnerState.Status, "REPORTED") &&
Review Comment:
Instead comparing against the string literal `"REPORTED"`, this should
compare against `string(tc.CacheStatusReported)` or cast `partnerState.Status`
to a `tc.CacheStatus` and compare it:
https://github.com/apache/trafficcontrol/blob/3713e3b990fbea03c40a580da8e58b81914741b5/lib/go-tc/enum.go#L381
##########
traffic_monitor/cache/stats_over_http.go:
##########
@@ -72,6 +74,14 @@ func statsOverHTTPParse(cacheName string, data io.Reader,
pollCTX interface{}) (
ctx := pollCTX.(*poller.HTTPPollCtx)
+ via := ctx.HTTPHeader.Get(rfc.Via)
+ if via != "" {
+ result := regexp.MustCompile(` ([^.]+)`).FindStringSubmatch(via)
Review Comment:
Same question here. Can `result` be renamed to something clearer?
##########
traffic_monitor/todata/todata.go:
##########
@@ -342,6 +346,48 @@ func getServerTypes(mc tc.TrafficMonitorConfigMap)
(map[tc.CacheName]tc.CacheTyp
return serverTypes, nil
}
+// getServerPartners gets the cache parners that have the same VIP
+func getServerParners(mc tc.TrafficMonitorConfigMap)
map[tc.CacheName]map[tc.CacheName]bool {
+ serverPartners := map[tc.CacheName]map[tc.CacheName]bool{}
Review Comment:
Since `ServerPartners` was renamed to `SameIpServers` in some places in
2e553a404f, this should stick to the same naming scheme.
##########
traffic_monitor/datareq/crstate.go:
##########
@@ -84,10 +88,58 @@ func filterDirectlyPolledCaches(crstates tc.CRStates)
tc.CRStates {
return filtered
}
-func srvTRStateSelf(localStates peer.CRStatesThreadsafe, directlyPolledOnly
bool) ([]byte, error) {
+func srvTRStateSelf(localStates peer.CRStatesThreadsafe, directlyPolledOnly
bool, toData todata.TODataThreadsafe) ([]byte, error) {
if !directlyPolledOnly {
- return tc.CRStatesMarshall(localStates.Get())
+ localStatesC := updateStatusAnycast(localStates, toData)
+ return tc.CRStatesMarshall(localStatesC)
}
- unfiltered := localStates.Get()
+ unfiltered := updateStatusAnycast(localStates, toData)
return tc.CRStatesMarshall(filterDirectlyPolledCaches(unfiltered))
}
+
+func updateStatusAnycast(localStates peer.CRStatesThreadsafe, toData
todata.TODataThreadsafe) tc.CRStates {
+ localStatesC := localStates.Get()
+ toDataC := toData.Get()
+
+ for cache, _ := range localStatesC.Caches {
+ if _, ok := toDataC.SameIpServers[cache]; ok {
+ // all server partners must be available if they are in
reported state
+ allAvailableV4 := true
+ allAvailableV6 := true
+ allIsAvailable := true
+ for partner, _ := range toDataC.SameIpServers[cache] {
+ if partnerState, ok :=
localStatesC.Caches[partner]; ok {
+ // a partner host is reported but is
marked down for too high traffic or load
+ // this host also needs to be marked
down to divert all traffic for their
+ // common anycast ip
+ if
strings.Contains(partnerState.Status, "REPORTED") &&
Review Comment:
Why use `string.Contains()`? Traffic Monitor usually compares
`CacheStatus`es like this:
https://github.com/apache/trafficcontrol/blob/3713e3b990fbea03c40a580da8e58b81914741b5/traffic_monitor/cache/cache.go#L217
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
