go-graphite / carbon-clickhouse

Graphite metrics receiver with ClickHouse as storage
MIT License
186 stars 47 forks source link

Uploader allocations #74

Closed msaf1980 closed 3 years ago

msaf1980 commented 3 years ago

Reduce allocations in uploader:

1) Modify key concatenation

- key := fmt.Sprintf("%d:%s", reader.Days(), unsafeString(name))
BenchmarkKeySprintf-6        8684433           130 ns/op          32 B/op          2 allocs/op
+ key := strconv.Itoa(int(reader.Days())) + ":" + unsafeString(name)
`BenchmarkKeyConcat-6       24734026            48.7 ns/op         4 B/op          1 allocs/op

2) Replace urlParse for tagParse (without map for tags list)

net.url.Parse
BenchmarkNetUrlParse-6       1500700           804 ns/op         592 B/op          7 allocs/op
urlParse
BenchmarkUrlParse-6          1000000          1000 ns/op         640 B/op          8 allocs/op
tagParse
BenchmarkTagParse-6          6899556           154 ns/op         112 B/op          2 allocs/op
lomik commented 3 years ago

I have new error after this changes. Carbon-clickhouse can't upload data to tagged table with error

[2020-10-29T17:03:57.172+0300] ERROR [upload] handle failed {“name”: “graphite_tagged”, “filename”: “/data/carbon-clickhouse/graphite_tagged/default.1603980221981854944”, “metrics”: 518092, “error”: “clickhouse response status 500: Code: 33, e.displayText() = DB::Exception: Cannot read all data. Bytes read: 185330033. Bytes expected: 199665097. (version 20.3.19.4 (official build))\n”, “time”: 3.351442262}

Last working commit 54763a7d040cce09ed8cc44baabc01225a003c97

lomik commented 3 years ago

18530e1bd4ee45c09603e56a5cdf6f050f0a8d64 reverted . Please find the problem and submit this improvement again

msaf1980 commented 3 years ago

I can't reproduce this. In our production environment this change work fine. Any additional info ? May be chunk file or metric name, that throw this error ?