mt-index-cat: support custom patterns and improve bench-realistic-workload-single-tenant.sh #1042

Dieterbe · 2018-09-11T13:11:39Z

No description provided.

replay · 2018-09-13T19:05:43Z

cmd/mt-index-cat/main.go

+		fmt.Println("         the chances need to add up to 100")
+		fmt.Println("         operation is one of:")
+		fmt.Println("         pass         (passthrough)")
+		fmt.Println("         <number>rcnw (replace number random consecutive nodes with wildcards")


that's a confusing sentence replace number random consecutive nodes with wildcards. I think i understand how it's meant, but hard to read it a few times.

how about replace <number> random consecutive nodes with wildcards ?

yeah, that would be much clearer because the number doesn't look like a word in the sentence

replay · 2018-09-13T19:23:25Z

cmd/mt-index-cat/out/template_functions.go

+
+// random choice between replacing a node with a wildcard, a char with a wildcard, and passthrough
+func pattern(in string) string {
+	mode := rand.Intn(3)


doesn't rand need a seed? otherwise it will keep repeating the same pattern on every run i think

mst@mst-nb1:~/abc$ cat main.go package main import "math/rand" import "fmt" func main() { fmt.Println(rand.Intn(10)) fmt.Println(rand.Intn(10)) fmt.Println(rand.Intn(10)) fmt.Println(rand.Intn(10)) fmt.Println(rand.Intn(10)) } mst@mst-nb1:~/abc$ go build main.go mst@mst-nb1:~/abc$ ./main 1 7 7 9 1 mst@mst-nb1:~/abc$ ./main 1 7 7 9 1 mst@mst-nb1:~/abc$ ./main 1 7 7 9 1

as this tool is used for repeated benchmark runs, it seems useful to have a consistent generated workload. no?

I guess you could say that, yeah...

we're not doing cryptography here so I think this is fine.

replay · 2018-09-13T19:43:21Z

cmd/mt-index-cat/out/template_functions.go

+		// and otherwise, sometimes valid patterns are produced,
+		// but it's also possible to produce patterns that won't match anything (if '.' was taken out)
+		chars := rand.Intn(5)
+		pos := rand.Intn(len(in) - chars)


if len(in) is <5 then it is possible that this will result be rand.Intn(<=0) (depending on chars). That would panic according to: https://golang.org/pkg/math/rand/#Intn

replay · 2018-09-13T19:51:00Z

cmd/mt-index-cat/out/template_functions.go

+
+// round rounds number d to the nearest r-boundary
+func round(d, r int64) int64 {
+	neg := d < 0


round appears to be prepared to deal with negative numbers as if they were positive numbers, but the list of else ifs in roundDuration appears to assume all numbers are positive. I think for consistency it would be better to either deal with negative numbers correctly or not everywhere.

roundDuration rounds durations. it seems to be implied to me that durations can't be negative.
i see your point though but i would rather not cripple a utility function because it's currently only being called by roundDuration.
we can argue both ways on this but it's not worth the time.

of course a duration can be negative. The sign tells you if the duration is the amount of time before or after the reference time.

Via a comment, at

metrictank/cmd/mt-index-cat/main.go

Line 192 in cda12b7

// set this after doing the query, to assure age can't possibly be negative

You state that age cant be negative, but that is not true. If the local time where this code is running is wrong, then age can easily be negative. Likewise, if the localtime on the server that is writing to the index is wrong the age can also end up negative.

Both of these are problems that we should not be masking.

But looking at the code, negatives already look to be handled.

true re clock wrong.
also callers of mt-index-cat could technically call the age() function on any integer, which is a 2nd reason how a duration could be negative.
while @woodsaj is correct that we don't seem to break upon negative durations, @replay also brings up a good point that roundDuration doesn't behave as expected when given negative numbers (it should round them also). i will fix

replay · 2018-09-13T20:19:22Z

cmd/mt-index-cat/out/tpl_pattern_custom.go

+		os.Exit(-1)
+	}
+
+	// we one or more of "<chance> <operation>" followed by an input string at the end.


i think the we wasn't supposed to be there

Dieterbe · 2018-09-14T10:15:29Z

@replay thank you. PTAL

Dieterbe · 2018-09-29T07:25:47Z

PR has been stuck for 2 weeks...
can someone please approve this

woodsaj · 2018-10-01T07:21:42Z

@Dieterbe can you rebase first.

woodsaj · 2018-10-01T07:51:44Z

cmd/mt-index-cat/out/tpl_pattern_custom.go

+func patternCustom(in ...interface{}) string {
+	usage := func() {
+		fmt.Println("usage of patternCustom:")
+		fmt.Println("input | patternCustom <chance> <operation>")


it needs to be made clear that <chance> <operation> can be repeated. convention for that is

input | patternCustom <chance> <operation>[ <chance> <operation>...]

woodsaj · 2018-10-01T08:01:34Z

cmd/mt-index-cat/out/tpl_pattern_custom.go

+
+// percentage chance, and function
+func patternCustom(in ...interface{}) string {
+	usage := func() {


This is duplicated in flag.Usage(). It should be split out to its own function to use for both?

patternCustomUsage(withExample bool) string { out := bytes.NewBuffer() out.WriteString(fmt.Sprintln("patternCustom: transforms a graphite.style.metric.name into a pattern with wildcards inserted according to rules provided:")) ... return out.String() }

woodsaj · 2018-10-01T08:17:03Z

cmd/mt-index-cat/out/tpl_pattern_custom.go

+		fmt.Println("the chances need to add up to 100")
+		fmt.Println("operation is one of:")
+		fmt.Println("       pass         (passthrough)")
+		fmt.Println("       <number>rcnw (replace <number> random consecutive nodes with wildcards")


consecutive nodes starting from where?

Do you actually mean

replace <number> consecutive nodes, from a random position with, a wildcard

what happens if the number of replacements is > than the (numberOfNodes - startPos)?

It also needs to be made clear that <number>is a single digit

thanks, i clarified this.
if there's not enough nodes/characters, mt-index-cat will error out. that behavior doesn't need to be explicitly mentioned in the usage though.

woodsaj · 2018-10-01T08:24:39Z

cmd/mt-index-cat/out/tpl_pattern_custom.go

+		fmt.Println("operation is one of:")
+		fmt.Println("       pass         (passthrough)")
+		fmt.Println("       <number>rcnw (replace <number> random consecutive nodes with wildcards")
+		fmt.Println("       <number>rccw (replace <number> random consecutive characters with wildcards")


Are the consecutive characters contained within a single node or will the potentially span multiple nodes.
what if the number of replacements is > (totalChars - startChar)?

it is not node aware. i suppose someone could add a node aware version if the need arises.
if the number is too high, see previous comment.

use 1 simple pattern instead of a mix of various patterns. we simply don't do enough requests to have that become a consistent pattern. Also, we have to filter for which metrics to use. Without filtering we would get all metrics back, e.g.: 1 some.id.of.a.metric.1 1 some.id.of.a.metric.10 1 some.id.of.a.metric.100 1 some.id.of.a.metric.1000 1 some.id.of.a.metric.10000 1 some.id.of.a.metric.100000 ... 1 some.id.of.a.metric.99999 This meant replacing a single char with a '*' could match many more series then we meant to. e.g. the top one, replacing 1 with * would match ALL series, resulting in very unfair benchmark runs. Now we know that we only ever query for 1 or 10 series.

Dieterbe · 2018-10-10T12:05:27Z

should be good to merge now

replay · 2018-10-17T18:41:05Z

benchmarks/realistic-workload-single-tenant.sh

+# this selects exactly 10k series that will match the regex, out of which we randomly replace 1 char with a wildcard, resulting in queries usually for 1 series, and sometimes for 10 series (depending on whether the replaced char falls within the dynamic part an the end of the name or not)
+# so 20/25 chances for 1 series, 5/25 chances for 10
+# as we execute 100Hz*300s=30k requests, this should give us a plenty high cache hit rate (while still testing the uncached code path)
+# in practice, the cache rate sometimes looks fairly low and i'm not sure why. but anyway (seeing about 15% hit partial and the rest are misses), and only in the last minute


that sentence is strange but anyway (seeing about 15% hit partial and the rest are misses), and only in the last minute. what is in the last minute, those hits/misses?

Suggested change

# in practice, the cache rate sometimes looks fairly low and i'm not sure why. but anyway (seeing about 15% hit partial and the rest are misses), and only in the last minute

# in practice, the cache rate sometimes looks fairly low and i'm not sure why (seeing about 15% hit partial and the rest are misses).

replay

👍

replay reviewed Sep 13, 2018

View reviewed changes

woodsaj reviewed Oct 1, 2018

View reviewed changes

Dieterbe added 14 commits October 10, 2018 13:53

tools don't need readme's. we have docs/tools.md

982c63f

organize output code better

e7fa41e

clearer

69f49bc

add patternCustom

57cd5d6

remove incorrect label

1f30fee

make the workload more realistic

3e01629

./scripts/dev/tools-to-doc.sh > docs/tools.md

b3b66d0

allow filtering by regex

42d145c

safety checks

565bae4

typo

ac2296b

clearer

4546fbb

also account for (round) negative durations

3e0a3d4

usage tweaks

f104ca7

Dieterbe force-pushed the better-mt-index-cat branch from cda12b7 to f104ca7 Compare October 10, 2018 11:59

replay reviewed Oct 17, 2018

View reviewed changes

replay approved these changes Oct 17, 2018

View reviewed changes

Dieterbe merged commit e4db0a4 into master Oct 18, 2018

Dieterbe deleted the better-mt-index-cat branch October 29, 2018 09:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mt-index-cat: support custom patterns and improve bench-realistic-workload-single-tenant.sh #1042

mt-index-cat: support custom patterns and improve bench-realistic-workload-single-tenant.sh #1042

Dieterbe commented Sep 11, 2018

replay Sep 13, 2018

Dieterbe Sep 14, 2018

replay Sep 14, 2018

replay Sep 13, 2018

replay Sep 13, 2018

Dieterbe Sep 14, 2018

replay Sep 14, 2018

Dieterbe Oct 10, 2018

replay Sep 13, 2018 •

edited

Loading

replay Sep 13, 2018 •

edited

Loading

Dieterbe Sep 14, 2018

woodsaj Oct 1, 2018

woodsaj Oct 1, 2018

Dieterbe Oct 10, 2018

replay Sep 13, 2018

Dieterbe commented Sep 14, 2018

Dieterbe commented Sep 29, 2018

woodsaj commented Oct 1, 2018

woodsaj Oct 1, 2018

woodsaj Oct 1, 2018

woodsaj Oct 1, 2018

woodsaj Oct 1, 2018 •

edited

Loading

woodsaj Oct 1, 2018 •

edited

Loading

Dieterbe Oct 10, 2018 •

edited

Loading

woodsaj Oct 1, 2018

Dieterbe Oct 10, 2018

Dieterbe commented Oct 10, 2018

replay Oct 17, 2018

replay left a comment

	# in practice, the cache rate sometimes looks fairly low and i'm not sure why. but anyway (seeing about 15% hit partial and the rest are misses), and only in the last minute
	# in practice, the cache rate sometimes looks fairly low and i'm not sure why (seeing about 15% hit partial and the rest are misses).

mt-index-cat: support custom patterns and improve bench-realistic-workload-single-tenant.sh #1042

mt-index-cat: support custom patterns and improve bench-realistic-workload-single-tenant.sh #1042

Conversation

Dieterbe commented Sep 11, 2018

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

replay Sep 13, 2018 • edited Loading

Choose a reason for hiding this comment

replay Sep 13, 2018 • edited Loading

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

Dieterbe commented Sep 14, 2018

Dieterbe commented Sep 29, 2018

woodsaj commented Oct 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woodsaj Oct 1, 2018 • edited Loading

Choose a reason for hiding this comment

woodsaj Oct 1, 2018 • edited Loading

Choose a reason for hiding this comment

Dieterbe Oct 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dieterbe commented Oct 10, 2018

Choose a reason for hiding this comment

replay left a comment

Choose a reason for hiding this comment

replay Sep 13, 2018 •

edited

Loading

replay Sep 13, 2018 •

edited

Loading

woodsaj Oct 1, 2018 •

edited

Loading

woodsaj Oct 1, 2018 •

edited

Loading

Dieterbe Oct 10, 2018 •

edited

Loading