You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficcontrol.apache.org by ne...@apache.org on 2016/12/08 18:46:06 UTC
[04/20] incubator-trafficcontrol git commit: Fix TM2 unnecessary
mutexed copies
Fix TM2 unnecessary mutexed copies
Project: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/commit/c80be36f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/tree/c80be36f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/diff/c80be36f
Branch: refs/heads/master
Commit: c80be36ff0d6563f5b447c76283beebffb9e2119
Parents: e41a745
Author: Robert Butts <ro...@gmail.com>
Authored: Mon Nov 28 14:30:13 2016 -0700
Committer: Dave Neuman <ne...@apache.org>
Committed: Thu Dec 8 11:44:33 2016 -0700
----------------------------------------------------------------------
.../traffic_monitor/manager/healthresult.go | 4 ++--
.../traffic_monitor/manager/monitorconfig.go | 11 +++++------
.../experimental/traffic_monitor/peer/crstates.go | 15 ++++++++-------
3 files changed, 15 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/c80be36f/traffic_monitor/experimental/traffic_monitor/manager/healthresult.go
----------------------------------------------------------------------
diff --git a/traffic_monitor/experimental/traffic_monitor/manager/healthresult.go b/traffic_monitor/experimental/traffic_monitor/manager/healthresult.go
index 67f844a..b8c46f7 100644
--- a/traffic_monitor/experimental/traffic_monitor/manager/healthresult.go
+++ b/traffic_monitor/experimental/traffic_monitor/manager/healthresult.go
@@ -229,7 +229,7 @@ func processHealthResult(
healthHistoryCopy[healthResult.ID] = pruneHistory(append([]cache.Result{healthResult}, healthHistoryCopy[healthResult.ID]...), maxHistory)
isAvailable, whyAvailable := health.EvalCache(healthResult, &monitorConfigCopy)
- if localStates.Get().Caches[healthResult.ID].IsAvailable != isAvailable {
+ if available, ok := localStates.GetCache(healthResult.ID); !ok || available.IsAvailable != isAvailable {
log.Infof("Changing state for %s was: %t now: %t because %s error: %v", healthResult.ID, prevResult.Available, isAvailable, whyAvailable, healthResult.Error)
events.Add(Event{Time: time.Now().Unix(), Description: whyAvailable, Name: healthResult.ID, Hostname: healthResult.ID, Type: toDataCopy.ServerTypes[healthResult.ID].String(), Available: isAvailable})
}
@@ -267,7 +267,7 @@ func calculateDeliveryServiceState(deliveryServiceServers map[enum.DeliveryServi
deliveryServiceState.IsAvailable = false
deliveryServiceState.DisabledLocations = []enum.CacheName{} // it's important this isn't nil, so it serialises to the JSON `[]` instead of `null`
for _, server := range deliveryServiceServers[deliveryServiceName] {
- if states.GetCache(server).IsAvailable {
+ if available, _ := states.GetCache(server); available.IsAvailable {
deliveryServiceState.IsAvailable = true
} else {
deliveryServiceState.DisabledLocations = append(deliveryServiceState.DisabledLocations, server)
http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/c80be36f/traffic_monitor/experimental/traffic_monitor/manager/monitorconfig.go
----------------------------------------------------------------------
diff --git a/traffic_monitor/experimental/traffic_monitor/manager/monitorconfig.go b/traffic_monitor/experimental/traffic_monitor/manager/monitorconfig.go
index 5c343ae..5307382 100644
--- a/traffic_monitor/experimental/traffic_monitor/manager/monitorconfig.go
+++ b/traffic_monitor/experimental/traffic_monitor/manager/monitorconfig.go
@@ -8,9 +8,9 @@ package manager
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
- *
+ *
* http://www.apache.org/licenses/LICENSE-2.0
- *
+ *
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -19,7 +19,6 @@ package manager
* under the License.
*/
-
import (
"fmt"
"strings"
@@ -219,7 +218,7 @@ func monitorConfigListen(
continue
}
// seed states with available = false until our polling cycle picks up a result
- if _, exists := localStates.Get().Caches[cacheName]; !exists {
+ if _, exists := localStates.GetCache(cacheName); !exists {
localStates.SetCache(cacheName, peer.IsAvailable{IsAvailable: false})
}
@@ -267,11 +266,11 @@ func monitorConfigListen(
// TODO because there are multiple writers to localStates.DeliveryService, there is a race condition, where MonitorConfig (this func) and HealthResultManager could write at the same time, and the HealthResultManager could overwrite a delivery service addition or deletion here. Probably the simplest and most performant fix would be a lock-free algorithm using atomic compare-and-swaps.
for _, ds := range monitorConfig.DeliveryService {
// since caches default to unavailable, also default DS false
- if _, exists := localStates.Get().Deliveryservice[enum.DeliveryServiceName(ds.XMLID)]; !exists {
+ if _, exists := localStates.GetDeliveryService(enum.DeliveryServiceName(ds.XMLID)); !exists {
localStates.SetDeliveryService(enum.DeliveryServiceName(ds.XMLID), peer.Deliveryservice{IsAvailable: false, DisabledLocations: []enum.CacheName{}}) // important to initialize DisabledLocations, so JSON is `[]` not `null`
}
}
- for ds := range localStates.Get().Deliveryservice {
+ for ds := range localStates.GetDeliveryServices() {
if _, exists := monitorConfig.DeliveryService[string(ds)]; !exists {
localStates.DeleteDeliveryService(ds)
}
http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/c80be36f/traffic_monitor/experimental/traffic_monitor/peer/crstates.go
----------------------------------------------------------------------
diff --git a/traffic_monitor/experimental/traffic_monitor/peer/crstates.go b/traffic_monitor/experimental/traffic_monitor/peer/crstates.go
index 65b7eb3..ef5176a 100644
--- a/traffic_monitor/experimental/traffic_monitor/peer/crstates.go
+++ b/traffic_monitor/experimental/traffic_monitor/peer/crstates.go
@@ -109,7 +109,6 @@ func NewCRStatesThreadsafe() CRStatesThreadsafe {
}
// Get returns the internal Crstates object for reading.
-// TODO add GetCaches, GetDeliveryservices?
func (t *CRStatesThreadsafe) Get() Crstates {
t.m.RLock()
defer t.m.RUnlock()
@@ -125,10 +124,11 @@ func (t *CRStatesThreadsafe) GetDeliveryServices() map[enum.DeliveryServiceName]
}
// GetCache returns the availability data of the given cache. This does not mutate, and is thus safe for multiple goroutines to call.
-func (t *CRStatesThreadsafe) GetCache(name enum.CacheName) IsAvailable {
+func (t *CRStatesThreadsafe) GetCache(name enum.CacheName) (available IsAvailable, ok bool) {
t.m.RLock()
- defer t.m.RUnlock()
- return t.crStates.Caches[name]
+ available, ok = t.crStates.Caches[name]
+ t.m.RUnlock()
+ return
}
// GetCaches returns the availability data of all caches. This does not mutate, and is thus safe for multiple goroutines to call.
@@ -139,10 +139,11 @@ func (t *CRStatesThreadsafe) GetCaches() map[enum.CacheName]IsAvailable {
}
// GetDeliveryService returns the availability data of the given delivery service. This does not mutate, and is thus safe for multiple goroutines to call.
-func (t *CRStatesThreadsafe) GetDeliveryService(name enum.DeliveryServiceName) Deliveryservice {
+func (t *CRStatesThreadsafe) GetDeliveryService(name enum.DeliveryServiceName) (ds Deliveryservice, ok bool) {
t.m.RLock()
- defer t.m.RUnlock()
- return t.crStates.Deliveryservice[name]
+ ds, ok = t.crStates.Deliveryservice[name]
+ t.m.RUnlock()
+ return
}
// SetCache sets the internal availability data for a particular cache.