zrhoffman commented on code in PR #7302:
URL: https://github.com/apache/trafficcontrol/pull/7302#discussion_r1105154898
##########
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 != "" {
+ viaRegexSubmatch := regexp.MustCompile(`
([^.]+)`).FindStringSubmatch(via)
Review Comment:
`([^.]+)` matches lots of characters that are not valid in a subdomain like
`,` and `\`. A regex for a subdomain should look like this:
https://github.com/apache/trafficcontrol/blob/dad57721e2a82ad0fb5eb1d30ba2ca33542d68ec/vendor/github.com/go-ozzo/ozzo-validation/is/rules.go#L130
##########
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 != "" {
+ viaRegexSubmatch := regexp.MustCompile(`
([^.]+)`).FindStringSubmatch(via)
Review Comment:
Same here, the compiled regex should be reused, rather than calling
`regexp.MustCompile()` each time.
##########
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 != "" {
+ viaRegexSubmatch := regexp.MustCompile(`
([^.]+)`).FindStringSubmatch(via)
+ if len(viaRegexSubmatch) > 0 {
+ astats.Ats[rfc.Via] = viaRegexSubmatch[1]
+ }
+ }
return stats, astats.Ats, nil
} else if ctype == "text/csv" {
+ via := ctx.HTTPHeader.Get(rfc.Via)
+ if via != "" {
+ viaRegexSubmatch := regexp.MustCompile(`
([^.]+)`).FindStringSubmatch(via)
Review Comment:
`regexp.MustCompile()` compiles the regex. The compiled regex should be
reused, rather than calling `regexp.MustCompile()` each time.
##########
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 := updateStatusSameIpServers(localStates, toData)
+ return tc.CRStatesMarshall(localStatesC)
}
- unfiltered := localStates.Get()
+ unfiltered := updateStatusSameIpServers(localStates, toData)
return tc.CRStatesMarshall(filterDirectlyPolledCaches(unfiltered))
}
+
+func updateStatusSameIpServers(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 servers with same ip 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 ip
+ if
strings.Contains(partnerState.Status, string(tc.CacheStatusReported)) &&
+
strings.Contains(partnerState.Status, "too high") {
Review Comment:
I don't love searching human-readable message for a specific string, there's
too many ways it could go wrong (what if human-readable part of the message
changes? What if the message changes so there's still a match but the meaning
is different, like `"Good thing it's not too high"`? What if, for some reason,
status messages get combined, so you get the first part without realizing it's
invalid because there is more than 1 message in the concatenated string).
Much more reliable would be to store the result of those comparisons so we
have a machine-readable way to know whether it's too high.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
