etcdctl: add initial check perf command #7591

xiang90 · 2017-03-24T00:28:28Z

This is a very simple performance validation command. The output is PASS or FAIL. There is nothing much to tune by design. If users want to find out more information, they should use benchmark tool instead.

example output:

ETCDCTL_API=3 ./etcdctl validate-perf --load=m
 60 / 60 Booooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo! 100.00%1m0s
PASS: Throughput is 968 writes/s
PASS: Slowest request took 0.490595s
PASS: Stddev is 0.030787s
PASS

I am going to finish up docs if this direction looks good. /cc @heyitsanthony @gyuho

xiang90 · 2017-03-24T00:35:57Z

My poor laptop failed large load test

ETCDCTL_API=3 ./etcdctl validate-perf --load=l
 60 / 60 Booooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo! 100.00%1m0s
FAIL: Throughput too low: 5007 writes/s
Slowest request took too long: 1.204120s
Stddev too high: 0.100497s
FAIL

gyuho · 2017-03-24T00:38:37Z

Is this for #6991?

xiang90 · 2017-03-24T00:39:02Z

@gyuho yes.

heyitsanthony · 2017-03-24T00:35:07Z

etcdctl/ctlv3/command/perf_validate.go

+// NewValidatePerfCommand returns the cobra command for "validate-perf".
+func NewValidatePerfCommand() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   "validate-perf [options]",


check-perf? check perf? perf check? perf? must be a better name..

perf seems ok. @gyuho @fanminshi any idea?

check-perf +1

If it's going to be check-perf, then check perf might be better. There's probably a few aspects of the cluster worth checking aside from only performance (e.g., check if running latest version, check if hashes match, ...) so check could cover all those as subcommands.

make sense to me.

check perf sounds good to me.

ok. will change.

heyitsanthony · 2017-03-24T00:37:56Z

etcdctl/ctlv3/command/perf_validate.go

+
+		for limit.Wait(ctx) == nil {
+			binary.PutVarint(k, int64(rand.Intn(math.MaxInt64)))
+			requests <- v3.OpPut(string(k), v)


I have a cluster and it looks like it's performing slow. So I run validate-perf to check if it's OK and now the cluster is full of garbage.

Force the command to take a prefix, check if the prefix is empty, and if so, put into the prefix and delete the whole thing on completion?

Good idea. My initial thought is that this tool should be used pre installation. But we can make it better with very little effort.

heyitsanthony · 2017-03-24T00:40:26Z

etcdctl/ctlv3/command/perf_validate.go

+	if ok {
+		fmt.Println("PASS")
+	} else {
+		fmt.Println("FAIL")


xiang90 · 2017-03-24T20:28:43Z

@heyitsanthony PTAL.

heyitsanthony · 2017-03-24T20:55:19Z

etcdctl/ctlv3/command/perf_validate.go

+
+	// TODO: support customized configuration
+	cmd.Flags().StringVar(&checkPerfLoad, "load", "s", "The workload to check the performance. Options are: s(small), m(medium), l(large), xl(xLarge).")
+	cmd.Flags().StringVar(&checkPerfPrefix, "prefix", "/etcdctl-check-perf/", "The key prefix used to check the performance.")


The prefix for writing the performance check's keys?

heyitsanthony · 2017-03-24T20:56:00Z

etcdctl/ctlv3/command/perf_validate.go

+	}
+
+	// TODO: support customized configuration
+	cmd.Flags().StringVar(&checkPerfLoad, "load", "s", "The workload to check the performance. Options are: s(small), m(medium), l(large), xl(xLarge).")


The performance check's workload model. Accepted workloads: s(small), m(medium), l(large), xl(xLarge)?

heyitsanthony · 2017-03-24T21:00:54Z

etcdctl/ctlv3/command/perf_validate.go

+func NewCheckCommand() *cobra.Command {
+	cc := &cobra.Command{
+		Use:   "check <subcommand>",
+		Short: "check related commands",


commands for checking properties of the etcd cluster?

heyitsanthony · 2017-03-24T21:03:19Z

etcdctl/ctlv3/command/perf_validate.go

+	var cfg checkPerfCfg
+	switch checkPerfLoad {
+	case "s", "small":
+		cfg = checkPerfCfgMap["s"]


var checkPerfAlias = map[string]string {"s" : "s", "small" : "s", ...} ... cfg, ok := checkPerfAlias[checkPerfLoad] if !ok { ExitWithError(...) }

?

heyitsanthony · 2017-03-24T21:17:28Z

etcdctl/ctlv3/command/perf_validate.go

+	r := report.NewReport("%4.4f")
+	var wg sync.WaitGroup
+
+	for i := range clients {


wg.Add(len(clients)) before loop instead of wg.Add(1)?

heyitsanthony · 2017-03-24T21:19:40Z

etcdctl/ctlv3/command/perf_validate.go

+		ticker := time.NewTicker(time.Second)
+
+		for i := 0; i < cfg.duration; i++ {
+			<-ticker.C


just time.Sleep(time.Second)?

Yea. I was going to add some cancelation logic. But I guess it is not needed.

heyitsanthony · 2017-03-24T21:25:13Z

etcdctl/ctlv3/command/perf_validate.go

+	for i := 0; i < cfg.clients; i++ {
+		clients = append(clients, mustClientFromCmd(cmd))
+	}
+


Some kind of check to avoid clobbering prefixes?

resp, _ := clients[0].Range(context.Background(), checkPerfPrefix, v3.WithPrefix(), v3.WithLimit(1)) if len(resp.Kvs) > 0 { ExitWithError(ExitInvalidInput, fmt.Sprintf("prefix %q has keys. Delete with etcdctl del --prefix %s", checkPerfPrefix, checkPerfPrefix) }

codecov-io · 2017-03-25T01:59:34Z

Codecov Report

Merging #7591 into master will decrease coverage by 0.38%.
The diff coverage is 15.3%.

@@            Coverage Diff             @@
##           master    #7591      +/-   ##
==========================================
- Coverage   73.95%   73.56%   -0.39%     
==========================================
  Files         325      326       +1     
  Lines       26454    26552      +98     
==========================================
- Hits        19565    19534      -31     
- Misses       5441     5558     +117     
- Partials     1448     1460      +12

Impacted Files	Coverage Δ
etcdctl/ctlv3/ctl.go	`100% <100%> (ø)`	⬆️
etcdctl/ctlv3/command/check.go	`14.43% <14.43%> (ø)`
pkg/transport/timeout_conn.go	`80% <0%> (-20%)`	⬇️
clientv3/namespace/watch.go	`87.87% <0%> (-12.13%)`	⬇️
pkg/adt/interval_tree.go	`83.83% <0%> (-7.75%)`	⬇️
etcdserver/api/v3rpc/util.go	`52.77% <0%> (-2.78%)`	⬇️
mvcc/watchable_store.go	`84.49% <0%> (-1.56%)`	⬇️
rafthttp/peer.go	`90.07% <0%> (-1.53%)`	⬇️
pkg/netutil/netutil.go	`80.88% <0%> (-1.48%)`	⬇️
clientv3/balancer.go	`95.32% <0%> (-1.17%)`	⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4ab5e5...60d3375. Read the comment docs.

heyitsanthony

two nits, lgtm otherwise

heyitsanthony · 2017-03-27T20:07:01Z

etcdctl/ctlv3/command/check.go

+		ExitWithError(ExitError, err)
+	}
+	if len(resp.Kvs) > 0 {
+		ExitWithError(ExitInvalidInput, fmt.Errorf("prefix %q has keys. Delete with etcdctl del --prefix %s before check the performance.", checkPerfPrefix, checkPerfPrefix))


s/ before check the performance./first.?

heyitsanthony · 2017-03-27T20:13:21Z

etcdctl/ctlv3/command/check.go

+	}
+
+	var cfg checkPerfCfg
+	switch checkPerfAlias[checkPerfLoad] {


model, ok := checkPerfAlias[checkPerfLoad] if !ok { ExitWithError(ExitBadFeature, fmt.Errorf("unknown load option %v", checkPerfLoad)) } cfg := checkPerfCfgMap[model]

?

heyitsanthony · 2017-03-28T00:02:33Z

lgtm. Thanks!

xiang90 force-pushed the validate branch from f7e6b2b to c8444b9 Compare March 24, 2017 00:36

heyitsanthony suggested changes Mar 24, 2017

View reviewed changes

xiang90 force-pushed the validate branch 4 times, most recently from f82ff26 to 27aefe9 Compare March 24, 2017 20:28

xiang90 changed the title ~~etcdctl: add initial validate-perf command~~ etcdctl: add initial check perf command Mar 24, 2017

heyitsanthony suggested changes Mar 24, 2017

View reviewed changes

gyuho added the kind/release-note label Mar 25, 2017

gyuho added this to the v3.2.0 milestone Mar 25, 2017

xiang90 force-pushed the validate branch 2 times, most recently from 2ab354d to 62e9390 Compare March 25, 2017 03:06

heyitsanthony suggested changes Mar 27, 2017

View reviewed changes

xiang90 force-pushed the validate branch from 62e9390 to ae6b26a Compare March 27, 2017 23:13

etcdctl: add initial check perf command

60d3375

xiang90 force-pushed the validate branch from ae6b26a to 60d3375 Compare March 28, 2017 00:01

heyitsanthony approved these changes Mar 28, 2017

View reviewed changes

xiang90 merged commit 65ad91b into etcd-io:master Mar 28, 2017

xiang90 deleted the validate branch March 28, 2017 00:24

xiang90 mentioned this pull request Mar 28, 2017

cluster performance validation script #6991

Closed

gyuho removed the kind/need-release-note-v3.3 label Nov 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

etcdctl: add initial check perf command #7591

etcdctl: add initial check perf command #7591

xiang90 commented Mar 24, 2017

xiang90 commented Mar 24, 2017 •

edited

Loading

gyuho commented Mar 24, 2017

xiang90 commented Mar 24, 2017

heyitsanthony Mar 24, 2017

xiang90 Mar 24, 2017

gyuho Mar 24, 2017

heyitsanthony Mar 24, 2017

xiang90 Mar 24, 2017

fanminshi Mar 24, 2017

xiang90 Mar 24, 2017

heyitsanthony Mar 24, 2017

xiang90 Mar 24, 2017

heyitsanthony Mar 24, 2017

xiang90 Mar 24, 2017

xiang90 commented Mar 24, 2017

heyitsanthony Mar 24, 2017

heyitsanthony Mar 24, 2017

heyitsanthony Mar 24, 2017

heyitsanthony Mar 24, 2017

heyitsanthony Mar 24, 2017

heyitsanthony Mar 24, 2017

xiang90 Mar 24, 2017

heyitsanthony Mar 24, 2017

codecov-io commented Mar 25, 2017 •

edited

Loading

heyitsanthony left a comment

heyitsanthony Mar 27, 2017

heyitsanthony Mar 27, 2017

heyitsanthony commented Mar 28, 2017

etcdctl: add initial check perf command #7591

etcdctl: add initial check perf command #7591

Conversation

xiang90 commented Mar 24, 2017

xiang90 commented Mar 24, 2017 • edited Loading

gyuho commented Mar 24, 2017

xiang90 commented Mar 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xiang90 commented Mar 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Mar 25, 2017 • edited Loading

Codecov Report

heyitsanthony left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heyitsanthony commented Mar 28, 2017

xiang90 commented Mar 24, 2017 •

edited

Loading

codecov-io commented Mar 25, 2017 •

edited

Loading