zrhoffman commented on code in PR #7302:
URL: https://github.com/apache/trafficcontrol/pull/7302#discussion_r1134499743
##########
traffic_monitor/datareq/crstate.go:
##########
@@ -21,6 +21,7 @@ package datareq
import (
"fmt"
+ "github.com/apache/trafficcontrol/traffic_monitor/health"
Review Comment:
Imports starting with `github.com/apache/trafficcontrol` should be grouped
separately
##########
traffic_monitor/cache/stats_over_http.go:
##########
@@ -56,6 +56,7 @@ const LOADAVG_SHIFT = 65536
func init() {
// AddStatsType("stats_over_http", statsParse, statsPrecompute)
Review Comment:
Can this commented line be removed?
##########
traffic_monitor/datareq/crstate.go:
##########
@@ -109,11 +110,11 @@ func updateStatusSameIpServers(localStates
peer.CRStatesThreadsafe, toData todat
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
+ // a partner host is reported but is
marked down for exceeding a threshold
// 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") {
+
strings.Contains(partnerState.Status, fmt.Sprint(health.TooHigh)) {
Review Comment:
`fmt.Sprint(health.TooHigh)` can be `health.TooHigh.String()`
##########
traffic_monitor/todata/todata.go:
##########
@@ -187,7 +187,9 @@ func (d TODataThreadsafe) Update(to
towrap.TrafficOpsSessionThreadsafe, cdn stri
return fmt.Errorf("getting server types from monitoring config:
%v", err)
}
- newTOData.SameIpServers = getSameIPServers(mc)
+ if val, ok := mc.Config["tm.sameipservers.control"]; ok &&
fmt.Sprint(val) == "true" {
Review Comment:
> `fmt.Sprint(val)`
Use `val.(string)`
##########
traffic_monitor/health/cache.go:
##########
@@ -46,6 +46,26 @@ const AvailableStr = "available"
// available to serve traffic.
const UnavailableStr = "unavailable"
+type Threshold int
+
+const (
+ NotEqual Threshold = iota
+ TooLow
+ TooHigh
+)
Review Comment:
Why not define a `Threshold` as a string?
```go
type Threshold string
const (
NotEqual Threshold = "too low"
TooLow = "too low"
TooHigh = "too high"
)
```
That would make converting to a string easier later.
##########
traffic_monitor/todata/todata.go:
##########
@@ -187,7 +187,9 @@ func (d TODataThreadsafe) Update(to
towrap.TrafficOpsSessionThreadsafe, cdn stri
return fmt.Errorf("getting server types from monitoring config:
%v", err)
}
- newTOData.SameIpServers = getSameIPServers(mc)
+ if val, ok := mc.Config["tm.sameipservers.control"]; ok &&
fmt.Sprint(val) == "true" {
Review Comment:
This is a new parameter. would you mind adding it to
https://traffic-control-cdn.readthedocs.io/en/latest/admin/traffic_monitor.html?
--
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
