functional-tester: decouple stresser from tester #6506
Conversation
|
@mitake Test failed? |
| Endpoint: m.grpcAddr(), | ||
| keyLargeSize: c.stressKeyLargeSize, | ||
| keySize: c.stressKeySize, | ||
| keySuffixRange: c.stressKeySuffixRange, | ||
| N: stressN, | ||
| rateLimiter: limiter, | ||
| rateLimiter: rate.NewLimiter(rate.Limit(c.stressQPS), c.stressQPS), |
There was a problem hiding this comment.
this changes the rate limiting behavior; if there are n stressers, the cumulative rate will be n*stressQPS instead of stressQPS
There was a problem hiding this comment.
Ah, this is incorrect as you pointed. I'll revert the part in the next update. Thanks.
| @@ -70,6 +71,7 @@ func main() { | |||
| stressKeyLargeSize: int(*stressKeyLargeSize), | |||
| stressKeySize: int(*stressKeySize), | |||
| stressKeySuffixRange: int(*stressKeySuffixRange), | |||
| stresserType: *stresserType, | |||
There was a problem hiding this comment.
passing a string around means cluster still needs to know a lot about stresser internals, how about tearing out most of the stresser stuff from cluster:
//stresser.go
type stressConfig struct {
qps int
keyLargeSize int
keySize int
keySuffixRange int
v2 bool
}
type stressBuilder func(m *member) Stresser
func newStressBuilder(s string, sc *stessConfig) stressBuilder {
switch s {
case "none": return func(*member) Stresser { return &nopStresser{} }
case "default":
l := rate.NewLimiter(rate.Limit(sc.QPS), sc.QPS)
return func(m *member) Stresser { ... }
}
}//main.go
c := &cluster{
...
stressBuilder: newStressBuilder(&sc, *stresserType)
...
}There was a problem hiding this comment.
The builder style seems to be good. I'll use it.
|
|
||
| func (s *nopStresser) Stress() error { return nil } | ||
| func (s *nopStresser) Cancel() {} | ||
| func (s *nopStresser) Report() (int, int) { return 0, 0 } |
There was a problem hiding this comment.
return time.Since(start).Seconds() * qps, 0? Otherwise failureUntilSnapshot will error out since it expects the success rate to be increasing.
There was a problem hiding this comment.
I didn't notice the check in failureUntilSnapshot, thanks for pointing!
|
@gyuho sorry, this PR has a format error. I'll fix it in the next update. |
This commit decouples stresser from the tester of functional-tester. For doing it, this commit adds a new option --stresser to etcd-tester. The option accepts two types of stresser: "default" and "nop". If the option is "default", etcd-tester stresses its etcd cluster with the existing stresser. If the option is "nop", etcd-tester does nothing for stressing. Partially fixes etcd-io#6446
mitake
force-pushed
the
decouple-stresser
branch
from
855d6f3
to
0e493c1
Compare
|
@gyuho @heyitsanthony updated for fixing the style problem and cleaner decoupling, PTAL |
|
lgtm. Thanks! |
This commit decouples stresser from the tester of
functional-tester. For doing it, this commit adds a new option
--stresser to etcd-tester. The option accepts two types of stresser:
"default" and "nop". If the option is "default", etcd-tester stresses
its etcd cluster with the existing stresser. If the option is "nop",
etcd-tester does nothing for stressing.
Partially fixes #6446
/cc @heyitsanthony