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 2017/02/20 01:57:11 UTC
[10/16] incubator-trafficcontrol git commit: Change TM2 TO wrapper to
assume thread safety
Change TM2 TO wrapper to assume thread safety
Changes trafficopswrapper to assume the Traffic Ops Session requests
are threadsafe, and only lock the Session pointer-pointer. This
should drastically reduce mutex contention for TO client requests.
Project: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/commit/7729a7e9
Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/tree/7729a7e9
Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/diff/7729a7e9
Branch: refs/heads/master
Commit: 7729a7e91d343e5fd1c0748cfc0f38b82ccb09b3
Parents: 7e46ee1
Author: Robert Butts <ro...@gmail.com>
Authored: Wed Feb 15 11:33:32 2017 -0700
Committer: Dave Neuman <ne...@apache.org>
Committed: Sun Feb 19 18:56:45 2017 -0700
----------------------------------------------------------------------
.../trafficopswrapper/trafficopswrapper.go | 108 +++++++++----------
1 file changed, 53 insertions(+), 55 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-trafficcontrol/blob/7729a7e9/traffic_monitor_golang/traffic_monitor/trafficopswrapper/trafficopswrapper.go
----------------------------------------------------------------------
diff --git a/traffic_monitor_golang/traffic_monitor/trafficopswrapper/trafficopswrapper.go b/traffic_monitor_golang/traffic_monitor/trafficopswrapper/trafficopswrapper.go
index 0bbc483..8acf61d 100644
--- a/traffic_monitor_golang/traffic_monitor/trafficopswrapper/trafficopswrapper.go
+++ b/traffic_monitor_golang/traffic_monitor/trafficopswrapper/trafficopswrapper.go
@@ -44,62 +44,72 @@ type ITrafficOpsSession interface {
var ErrNilSession = fmt.Errorf("nil session")
-func (s TrafficOpsSessionThreadsafe) URL() (string, error) {
+// TrafficOpsSessionThreadsafe provides access to the Traffic Ops client safe for multiple goroutines. This fulfills the ITrafficOpsSession interface.
+type TrafficOpsSessionThreadsafe struct {
+ session **to.Session // pointer-to-pointer, because we're given a pointer from the Traffic Ops package, and we don't want to copy it.
+ m *sync.Mutex
+}
+
+// NewTrafficOpsSessionThreadsafe returns a new threadsafe TrafficOpsSessionThreadsafe wrapping the given `Session`.
+func NewTrafficOpsSessionThreadsafe(s *to.Session) TrafficOpsSessionThreadsafe {
+ return TrafficOpsSessionThreadsafe{&s, &sync.Mutex{}}
+}
+
+// Set sets the internal Traffic Ops session. This is safe for multiple goroutines, being aware they will race.
+func (s TrafficOpsSessionThreadsafe) Set(session *to.Session) {
s.m.Lock()
defer s.m.Unlock()
- if s.session == nil || *s.session == nil {
- return "", ErrNilSession
- }
- url := (*s.session).URL
- return url, nil
+ *s.session = session
}
-func (s TrafficOpsSessionThreadsafe) User() (string, error) {
+// getThreadsafeSession is used internally to get a copy of the session pointer, or nil if it doesn't exist. This should not be used outside TrafficOpsSessionThreadsafe, and never stored, because part of the purpose of TrafficOpsSessionThreadsafe is to store a pointer to the Session pointer, so it can be updated by one goroutine and immediately used by another. This should only be called immediately before using the session, since someone else may update it concurrently.
+func (s TrafficOpsSessionThreadsafe) get() *to.Session {
s.m.Lock()
defer s.m.Unlock()
if s.session == nil || *s.session == nil {
- return "", ErrNilSession
+ return nil
}
- user := (*s.session).UserName
- return user, nil
+ return *s.session
}
-// TrafficOpsSessionThreadsafe provides access to the Traffic Ops client safe for multiple goroutines. This fulfills the ITrafficOpsSession interface.
-type TrafficOpsSessionThreadsafe struct {
- session **to.Session // pointer-to-pointer, because we're given a pointer from the Traffic Ops package, and we don't want to copy it.
- m *sync.Mutex
+func (s TrafficOpsSessionThreadsafe) URL() (string, error) {
+ ss := s.get()
+ if ss == nil {
+ return "", ErrNilSession
+ }
+ return ss.URL, nil
}
-// NewTrafficOpsSessionThreadsafe returns a new threadsafe TrafficOpsSessionThreadsafe wrapping the given `Session`.
-func NewTrafficOpsSessionThreadsafe(s *to.Session) TrafficOpsSessionThreadsafe {
- return TrafficOpsSessionThreadsafe{&s, &sync.Mutex{}}
+func (s TrafficOpsSessionThreadsafe) User() (string, error) {
+ ss := s.get()
+ if ss == nil {
+ return "", ErrNilSession
+ }
+ return ss.UserName, nil
}
// CRConfigRaw returns the CRConfig from the Traffic Ops. This is safe for multiple goroutines.
func (s TrafficOpsSessionThreadsafe) CRConfigRaw(cdn string) ([]byte, error) {
- s.m.Lock()
- defer s.m.Unlock()
- if s.session == nil || *s.session == nil {
+ ss := s.get()
+ if ss == nil {
return nil, ErrNilSession
}
- b, _, e := (*s.session).GetCRConfig(cdn)
+ b, _, e := ss.GetCRConfig(cdn)
return b, e
}
// TrafficMonitorConfigMapRaw returns the Traffic Monitor config map from the Traffic Ops, directly from the monitoring.json endpoint. This is not usually what is needed, rather monitoring needs the snapshotted CRConfig data, which is filled in by `TrafficMonitorConfigMap`. This is safe for multiple goroutines.
-func (s TrafficOpsSessionThreadsafe) TrafficMonitorConfigMapRaw(cdn string) (*to.TrafficMonitorConfigMap, error) {
- s.m.Lock()
- defer s.m.Unlock()
- if s.session == nil || *s.session == nil {
+func (s TrafficOpsSessionThreadsafe) trafficMonitorConfigMapRaw(cdn string) (*to.TrafficMonitorConfigMap, error) {
+ ss := s.get()
+ if ss == nil {
return nil, ErrNilSession
}
- d, e := (*s.session).TrafficMonitorConfigMap(cdn)
- return d, e
+ return ss.TrafficMonitorConfigMap(cdn)
}
// TrafficMonitorConfigMap returns the Traffic Monitor config map from the Traffic Ops. This is safe for multiple goroutines.
func (s TrafficOpsSessionThreadsafe) TrafficMonitorConfigMap(cdn string) (*to.TrafficMonitorConfigMap, error) {
- mc, err := s.TrafficMonitorConfigMapRaw(cdn)
+ mc, err := s.trafficMonitorConfigMapRaw(cdn)
if err != nil {
return nil, fmt.Errorf("getting monitor config map: %v", err)
}
@@ -177,54 +187,42 @@ func CreateMonitorConfig(crConfig crconfig.CRConfig, mc *to.TrafficMonitorConfig
return mc, nil
}
-// Set sets the internal Traffic Ops session. This is safe for multiple goroutines, being aware they will race.
-func (s TrafficOpsSessionThreadsafe) Set(session *to.Session) {
- s.m.Lock()
- defer s.m.Unlock()
- *s.session = session
-}
-
func (s TrafficOpsSessionThreadsafe) Servers() ([]to.Server, error) {
- s.m.Lock()
- defer s.m.Unlock()
- if s.session == nil || *s.session == nil {
+ ss := s.get()
+ if ss == nil {
return nil, ErrNilSession
}
- return (*s.session).Servers()
+ return ss.Servers()
}
func (s TrafficOpsSessionThreadsafe) Profiles() ([]to.Profile, error) {
- s.m.Lock()
- defer s.m.Unlock()
- if s.session == nil || *s.session == nil {
+ ss := s.get()
+ if ss == nil {
return nil, ErrNilSession
}
- return (*s.session).Profiles()
+ return ss.Profiles()
}
func (s TrafficOpsSessionThreadsafe) Parameters(profileName string) ([]to.Parameter, error) {
- s.m.Lock()
- defer s.m.Unlock()
- if s.session == nil || *s.session == nil {
+ ss := s.get()
+ if ss == nil {
return nil, ErrNilSession
}
- return (*s.session).Parameters(profileName)
+ return ss.Parameters(profileName)
}
func (s TrafficOpsSessionThreadsafe) DeliveryServices() ([]to.DeliveryService, error) {
- s.m.Lock()
- defer s.m.Unlock()
- if s.session == nil || *s.session == nil {
+ ss := s.get()
+ if ss == nil {
return nil, ErrNilSession
}
- return (*s.session).DeliveryServices()
+ return ss.DeliveryServices()
}
func (s TrafficOpsSessionThreadsafe) CacheGroups() ([]to.CacheGroup, error) {
- s.m.Lock()
- defer s.m.Unlock()
- if s.session == nil || *s.session == nil {
+ ss := s.get()
+ if ss == nil {
return nil, ErrNilSession
}
- return (*s.session).CacheGroups()
+ return ss.CacheGroups()
}