Refactor metric samples to use Atlas for tags and have time series #2594
Conversation
|
Hmm I'd definitely want to see more benchmarks and tests before we merge something like this 🤔 Considering we'll have just a single copy of the tags per Besides, I have just skimmed the atlas code, but I am not sure it works like how we'd need it to work. A tag set with keys |
We are definittely not just merging this. I would even expect that we would rewrite most of the code to actually not have a global root, but have it in the metrics.Registry before we even think of merging. I would also expect at least some of the code to actually use the root node directly (the http path definitely) instead of
How do we build and have only one copy and keep it only one copy? From the code that I have seen so far - we always build a new slice of tags and then we hash it and go get it from the registry. But we still made a new copy - we just threw it away in the mean time.
It should, but you can test it and open an issue if it doesn't ;) To be honest I would expect the reduction in memory to be negligable for most real uses case, but the reduction in GC time in those same cases should be significant once we are using this directly building from lib.State#Tags instead of going from the root node up for each set. One of the points of this structure is that it "memorizes" how it was constructed and optimizes the same route for the next time. So as long as the code that generates it takes the same route it will be faster the following times. But building it from a map has more chances of it being randomized. I still would like to see if even like that it makes a difference and in which direction if you just have a fairly basic test. I would expect thsi will be slightly ... worse, before we stop generating the whole map and instead use the atlas.Node for that as well |
codebien
force-pushed
the
atlas
branch
3 times, most recently
from
41823e4
to
d7225a4
Compare
codebien
force-pushed
the
atlas
branch
2 times, most recently
from
a31b1b6
to
d55c5d3
Compare
codebien
force-pushed
the
atlas
branch
4 times, most recently
from
1255ebb
to
1589807
Compare
|
Updates:
Note: as you can see from the failing xk6 test, we are breaking the extensions removing the ability to create a |
This implements the consensus from the PR comment (#issuecomment-1205273413), with some minor modifications.
This will allow metric Samples to be easily grouped with other Samples with the same metric and tags. It implements the consensus described in my second PR comment (#issuecomment-1205359198)
After the introduction of Atlas, we decided to keep `url` as a normal tag but it always set to have the same value of the `name` tag. In this way, if the `url` contains high-cardinality values it wouldn't affect the system. This is the same thing that currently the cloud output does.
Remove for a temporary commit the experimental module to allow bumping the k6 dependency in the extension's repository.
|
As a reminder, otherwise, the
|
Uses the xk6-websockets main version that is now migrated to the time series version.
Integrated https://github.com/mstoykov/atlas as the internal data structure for
SampleTags. Atlas is a Btree implementation based on[2]stringarray for the nodes (the stored tags pairs). It uses pointers most of the time for comparisons so it's very fast and read locks can be avoided.It allows allocating a specific Tag pair just once. The reduction in terms of memory from my basic benchmarks is very important. For example, the benchmark on a similar function reduces the overall allocated memory from 600MB to 87MB (note: this is a very extreme and limit case, the average is not so high).
Also, the time series concept has been introduced as defined from the Data model and Storage parts in #2580.
Closes #1831 #2580