-
Notifications
You must be signed in to change notification settings - Fork 99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
This adds a new load tester #332
Conversation
The load tester in the repo currently is not very configurable, yet still overly complicated. This introduces a completely fresh rewrite, which is both more configurable for testing different things, and simpler.
This makes it easier to leave the load generator running while gostatsd is restarting.
func parseArgs(args []string) commandOptions { | ||
var opts commandOptions | ||
parser := flags.NewParser(&opts, flags.HelpFlag | flags.PassDoubleDash) | ||
parser.LongDescription = "" + // because gofmt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use `` string? (I can't remember its name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Habit mainly. And it doesn't look good completely left aligned in the source, but if you add spaces then go-flags strips it. Either way, it's sub-optimal, so I went with the "embrace the suck" style.
} | ||
|
||
func (mg *metricGenerator) nextCounter(sb *strings.Builder) { | ||
atomic.AddUint64(&mg.counters.count, ^uint64(0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I ask why we are using the max unit64 value?
Is that just going to overflow the value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how you do an atomic subtract from an unsigned: ^0
=== max unsigned === -1
signed.
metricGenerators := make([]*metricGenerator, 0, opts.Workers) | ||
for i := uint(0); i < opts.Workers; i++ { | ||
generator := &metricGenerator{ | ||
rnd: rand.New(rand.NewSource(rand.Int63())), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:fry: What?
Would this be better to seed with time.Now.UnixNano()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
time.Now()
makes no guarantee of advancing when called rapidly in succession. In all fairness, rand.Int63()
makes no guarantee of not returning the same number twice in a row either.
|
||
func sendMetricsWorker( | ||
address string, | ||
bufSize uint, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type not needed for this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When every param is on its own line, it's neater. Also it means if params change then you get cleaner diffs, kind of like having a trailing ,
on the last parameter.
The load tester in the repo currently is not very configurable, yet still overly complicated. This introduces a completely fresh rewrite, which is both more configurable for testing different things, and simpler.
I'm a bit iffy on using go-flags, but I find it to be infinitely nicer than viper (and infinitely squared nicer than the golang flag library). Since there's no need for the more complicated options provided by viper, I went with it.