cmd/scollector: bugfix - datapoint validation #1856

Merged
merged 1 commit into from Aug 12, 2016

Conversation

Projects
None yet
2 participants
@mathpl
Contributor

mathpl commented Aug 9, 2016

  • TagSet.Clean() validates cleaned key/value are not empty.
  • DataPoint.Clean() is also called when quickly dequeuing the datapoint channel.
  • Data points deemed unclean increment scollector.collect.discarded instead of scollector.collect.dropped.
@@ -131,6 +135,13 @@ func InitChan(tsdbhost *url.URL, root string, ch chan *opentsdb.DataPoint) error
slock.Unlock()
return
})
+ Set("collect.discarded", Tags, func() (i interface{}) {
+ slock.Lock()

This comment has been minimized.

@captncraig

captncraig Aug 9, 2016

Contributor

I don't think you even need to lock if you are using the atomic operations. You could use atomic.LoadInt64 and not need locks at all. I wish we had done all these stats like that from the start.

@captncraig

captncraig Aug 9, 2016

Contributor

I don't think you even need to lock if you are using the atomic operations. You could use atomic.LoadInt64 and not need locks at all. I wish we had done all these stats like that from the start.

This comment has been minimized.

@mathpl

mathpl Aug 9, 2016

Contributor

want me to move all those to atomic.* ? I'd be happy to.

@mathpl

mathpl Aug 9, 2016

Contributor

want me to move all those to atomic.* ? I'd be happy to.

This comment has been minimized.

@captncraig

captncraig Aug 12, 2016

Contributor

Yes please. I'm going to merge this for now, but if you wanted to do that in a seperate pr it would be appreciated. Thanks!

@captncraig

captncraig Aug 12, 2016

Contributor

Yes please. I'm going to merge this for now, but if you wanted to do that in a seperate pr it would be appreciated. Thanks!

@captncraig captncraig merged commit 89acb98 into bosun-monitor:master Aug 12, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

gbrayut added a commit that referenced this pull request Aug 16, 2016

gbrayut added a commit that referenced this pull request Aug 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment