ocket8888 commented on a change in pull request #4916:
URL: https://github.com/apache/trafficcontrol/pull/4916#discussion_r466648588
##########
File path: traffic_monitor/threadsafe/resultstathistory.go
##########
@@ -55,55 +64,212 @@ func (h *ResultInfoHistory) Get() cache.ResultInfoHistory {
return *h.history
}
-// Set sets the internal ResultInfoHistory. This is only safe for one thread
of execution. This MUST NOT be called from multiple threads.
+// Set sets the internal ResultInfoHistory. This is only safe for one thread of
+// execution. This MUST NOT be called from multiple threads.
func (h *ResultInfoHistory) Set(v cache.ResultInfoHistory) {
h.m.Lock()
*h.history = v
h.m.Unlock()
}
-type ResultStatHistory struct{ *sync.Map } //
map[tc.CacheName]map[interfaceName]ResultStatValHistory
+// ResultStatHistory is a thread-safe mapping of cache server hostnames to
+// CacheStatHistory objects containing statistics for those cache servers.
+type ResultStatHistory struct{ *sync.Map } // map[string]CacheStatHistory
+// NewResultStatHistory constructs a new, empty ResultStatHistory.
func NewResultStatHistory() ResultStatHistory {
return ResultStatHistory{&sync.Map{}}
}
-func (h ResultStatHistory) LoadOrStore(cache tc.CacheName)
map[string]ResultStatValHistory {
+// LoadOrStore returns the stored CacheStatHistory for the given cache server
+// hostname if it has already been stored. If it has not already been stored, a
+// new, empty CacheStatHistory object is created, stored under the given
+// hostname, and returned.
+func (h ResultStatHistory) LoadOrStore(hostname string) CacheStatHistory {
// TODO change to use sync.Pool?
- v, loaded := h.Map.LoadOrStore(cache, NewResultStatValHistory())
- if !loaded {
- v = map[string]ResultStatValHistory{}
- }
- if rv, ok := v.(ResultStatValHistory); ok {
- v =
map[string]ResultStatValHistory{tc.CacheInterfacesAggregate: rv}
+ v, _ := h.Map.LoadOrStore(hostname, NewCacheStatHistory())
+ rv, ok := v.(CacheStatHistory)
+ if !ok {
+ log.Errorf("Failed to load or store stat history for '%s':
invalid stored type.", hostname)
+ return NewCacheStatHistory()
}
- return v.(map[string]ResultStatValHistory)
+
+ return rv
}
-// Range behaves like sync.Map.Range. It calls f for every value in the map;
if f returns false, the iteration is stopped.
-func (h ResultStatHistory) Range(f func(cache tc.CacheName, interfaceName
string, val ResultStatValHistory) bool) {
+// Range behaves like sync.Map.Range. It calls f for every value in the map; if
+// f returns false, the iteration is stopped.
+func (h ResultStatHistory) Range(f func(cacheName string, val
CacheStatHistory) bool) {
h.Map.Range(func(k, v interface{}) bool {
- i, ok := v.(map[string]ResultStatValHistory)
+ i, ok := v.(CacheStatHistory)
if !ok {
- log.Warnln("Cannot umarshal result stat val history")
+ log.Warnf("Non-CacheStatHistory object (%T) found in
ResultStatHistory during Range.", v)
return true
}
- for a, b := range i {
- if !f(k.(tc.CacheName), a, b) {
- return false
- }
+ cacheName, ok := k.(string)
+ if !ok {
+ log.Warnf("Non-string object (%T) found as key in
ResultStatHistory during Range.", k)
+ return true
}
- return true
+ return f(cacheName, i)
})
}
-// ResultStatValHistory is threadsafe for one writer. Specifically, because a
CompareAndSwap is not provided, it's not possible to Load and Store without a
race condition.
-// If multiple writers were necessary, it wouldn't be difficult to add a
CompareAndSwap, internally storing an atomically-accessed pointer to the slice.
+// This is just a convenience structure used only for passing data about a
+// single statistic for a network interface into
+// compareAndAppendStatForInterface.
+type interfaceStat struct {
+ InterfaceName string
+ Stat interface{}
+ StatName string
+ Time time.Time
+}
+
+// This is a little helper function used to compare a single stat for a single
+// network interface to its historical values and do the appropriate appending
+// and management of the history to ensure it never exceeds `limit`.
+func compareAndAppendStatForInterface(history []cache.ResultStatVal, errs
strings.Builder, limit uint64, stat interfaceStat) []cache.ResultStatVal {
+ if history == nil {
+ history = make([]cache.ResultStatVal, 0, limit)
+ }
+
+ ok, err := newStatEqual(history, stat.Stat)
+ if err != nil {
+ errs.WriteString(stat.InterfaceName)
+ errs.Write([]byte(": cannot add stat "))
+ errs.WriteString(stat.StatName)
+ errs.Write([]byte(": "))
+ errs.WriteString(err.Error())
+ errs.Write([]byte("; "))
Review comment:
yeah, string building is up to a thousand times faster than
`fmt.Sprintf`, and TM is notoriously resource-hoggy
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
