From 8044402b4ebe89b5a27d525ffcaee733bc4dc963 Mon Sep 17 00:00:00 2001 From: Adrien Kohlbecker Date: Mon, 15 Oct 2018 14:28:31 +0100 Subject: [PATCH] Update runCounter to be int32 On 32 bits system like armv7 (raspberry pi), using `atomic.AddInt64` (and other variants) resulted in a nil pointer exception. I understand datadog does not officially support arm, but since the fix is pretty easy in this case, it seemed like a good candidate for a PR. This go issue has more details https://github.com/golang/go/issues/599 In our case we make limited use of 64 bits atomic functions, in two places: - `runCounter`: This is only used for logging and assuming an increment each second would overflow in 70 years if 32 bits, so is most likely safe - `realTimeEnabled`: This is used as a pseudo bolean so 0/1 and is safe I've recompiled the agent 6.5.2 from source using this fix and I'm successfully running it on my Raspberry Pi, with processes reporting. --- cmd/agent/collector.go | 14 +++++++------- cmd/agent/collector_test.go | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/agent/collector.go b/cmd/agent/collector.go index d54f070da..83bd4b15c 100644 --- a/cmd/agent/collector.go +++ b/cmd/agent/collector.go @@ -31,14 +31,14 @@ type Collector struct { cfg *config.AgentConfig httpClient http.Client groupID int32 - runCounter int64 + runCounter int32 enabledChecks []checks.Check // Controls the real-time interval, can change live. realTimeInterval time.Duration // Set to 1 if enabled 0 is not. We're using an integer // so we can use the sync/atomic for thread-safe access. - realTimeEnabled int64 + realTimeEnabled int32 } // NewCollector creates a new Collector @@ -71,7 +71,7 @@ func NewCollector(cfg *config.AgentConfig) (Collector, error) { } func (l *Collector) runCheck(c checks.Check) { - runCounter := atomic.AddInt64(&l.runCounter, 1) + runCounter := atomic.AddInt32(&l.runCounter, 1) s := time.Now() // update the last collected timestamp for info updateLastCollectTime(time.Now()) @@ -139,7 +139,7 @@ func (l *Collector) run(exit chan bool) { for { select { case <-ticker.C: - realTimeEnabled := atomic.LoadInt64(&l.realTimeEnabled) == 1 + realTimeEnabled := atomic.LoadInt32(&l.realTimeEnabled) == 1 if !c.RealTime() || realTimeEnabled { l.runCheck(c) } @@ -212,7 +212,7 @@ func (l *Collector) postMessage(checkPath string, m model.MessageBody) { } func (l *Collector) updateStatus(statuses []*model.CollectorStatus) { - curEnabled := atomic.LoadInt64(&l.realTimeEnabled) == 1 + curEnabled := atomic.LoadInt32(&l.realTimeEnabled) == 1 // If any of the endpoints wants real-time we'll do that. // We will pick the maximum interval given since generally this is @@ -229,10 +229,10 @@ func (l *Collector) updateStatus(statuses []*model.CollectorStatus) { if curEnabled && !shouldEnableRT { log.Info("Detected 0 clients, disabling real-time mode") - atomic.StoreInt64(&l.realTimeEnabled, 0) + atomic.StoreInt32(&l.realTimeEnabled, 0) } else if !curEnabled && shouldEnableRT { log.Info("Detected active clients, enabling real-time mode") - atomic.StoreInt64(&l.realTimeEnabled, 1) + atomic.StoreInt32(&l.realTimeEnabled, 1) } if maxInterval != l.realTimeInterval { diff --git a/cmd/agent/collector_test.go b/cmd/agent/collector_test.go index dd8da55f6..65619b3ef 100644 --- a/cmd/agent/collector_test.go +++ b/cmd/agent/collector_test.go @@ -25,7 +25,7 @@ func TestUpdateRTStatus(t *testing.T) { {ActiveClients: 0, Interval: 2}, } c.updateStatus(statuses) - assert.Equal(int64(1), atomic.LoadInt64(&c.realTimeEnabled)) + assert.Equal(int32(1), atomic.LoadInt32(&c.realTimeEnabled)) // Validate that we stay that way statuses = []*model.CollectorStatus{ @@ -34,7 +34,7 @@ func TestUpdateRTStatus(t *testing.T) { {ActiveClients: 0, Interval: 2}, } c.updateStatus(statuses) - assert.Equal(int64(1), atomic.LoadInt64(&c.realTimeEnabled)) + assert.Equal(int32(1), atomic.LoadInt32(&c.realTimeEnabled)) // And that it can turn back off statuses = []*model.CollectorStatus{ @@ -43,7 +43,7 @@ func TestUpdateRTStatus(t *testing.T) { {ActiveClients: 0, Interval: 2}, } c.updateStatus(statuses) - assert.Equal(int64(0), atomic.LoadInt64(&c.realTimeEnabled)) + assert.Equal(int32(0), atomic.LoadInt32(&c.realTimeEnabled)) } func TestUpdateRTInterval(t *testing.T) { @@ -61,6 +61,6 @@ func TestUpdateRTInterval(t *testing.T) { {ActiveClients: 0, Interval: 10}, } c.updateStatus(statuses) - assert.Equal(int64(1), atomic.LoadInt64(&c.realTimeEnabled)) + assert.Equal(int32(1), atomic.LoadInt32(&c.realTimeEnabled)) assert.Equal(10*time.Second, c.realTimeInterval) }