A metrics scrape cloned every resource to read a few fields off it - #524
Merged
Conversation
The scrape reports how many tables, namespaces and proxy groups there are and
how many are in each state, and nothing else about them. It got those numbers
by cloning the whole set of each -- every name, every serving option -- then
calling .len() on it and filtering it three times.
At 4096 tables that was 648.5us for the tables and 292.3us for the namespaces
out of a 1093.3us scrape: most of the answer spent building lists that were
counted and thrown away.
Counting them in place instead:
namespaces tables before after
32 1024 288.9us 8.0us
64 4096 2796.2us 18.4us
The scrape is now close to flat in the size of the cluster rather than growing
with it, which matters because it runs on a timer.
Servers and proxies are still listed. They are the two the scrape actually
itemises -- per-server capacity, per-proxy heartbeat age -- so those clones are
used.
The three states are the whole of MetaEntityState, so a tally's total is the
count the listing's .len() used to give. A state name that is not one of the
three counts as none, which is what filtering for it used to produce.
Verified by generating the entire scrape for a cluster holding tables,
namespaces and proxy groups in all three states, before and after: all 115
lines are identical.
Test: the tallies are checked against listing each resource and counting those,
for every state and for the total, with the states deliberately spread so the
agreement cannot be every count being zero.
Checked against a mutation: with frozen counted as normal the test fails.
Measured with the arms interleaved, and the changed arm run twice.
A server record carries its shard loads, its stat loads and its shard serving
states -- one entry per shard the node holds, and the serving states carry
strings. A scrape reads none of them. It wants each server's address, its state
and five counters, and each proxy's address, state and restart count.
Listing the servers to get there cloned everything else too, so the scrape cost
what the cluster was holding rather than how many nodes it had:
servers shards each before after
8 250 256.1us 11.6us
32 250 1026.0us 29.8us
32 1000 4714.8us 30.4us
The whole report is now taken in one pass, which also means one acquisition of
the read lock where there were five -- so the numbers in a scrape describe one
moment rather than five consecutive ones.
Servers and proxies are still reported one by one. Only the fields nothing
reads are gone.
Verified by generating the entire scrape for a cluster with four servers that
have heartbeated with shard loads and serving states, three proxies, twelve
tables and proxy groups, across all three states: 136 lines, 27 of them naming
a server or a proxy, identical before and after.
The row check is asserted against the full server records field by field, and
guarded by a check that some server actually reported a non-zero count -- which
caught the first version of this test, where the records were reported through
the wrong field and every counter was zero.
bjmeetsfo
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A metrics scrape reports five kinds of resource. Tables, namespaces and proxy groups it reports only by number and by state. Servers and proxies it reports one by one, but by a handful of fields each.
It got all of that by listing every resource in full: cloning each table with its name and serving options, and each server with its shard loads, its stat loads and its shard serving states -- one entry per shard the node holds, with strings inside. Then it counted them, or read three fields off them.
So the scrape cost what the cluster was holding.
It is now close to flat in the size of the cluster, which matters because it runs on a timer.
One pass, one lock
The scrape made five separate listing calls, each taking the read lock again, so its numbers described five consecutive moments. It now takes one report under one acquisition.
What is still reported per resource
Servers and proxies, one by one, exactly as before. Only the fields nothing reads are gone: the per-shard loads, stat loads and serving states, and the numa nodes.
The three states are the whole of
MetaEntityState, so a tally's total is the count the listing's.len()used to give. A state name that is not one of the three counts as none, which is what filtering for it used to produce.Verification
The entire scrape was generated for a cluster with four servers that have heartbeated with shard loads and serving states, three proxies, twelve tables, two proxy groups, and resources in all three states -- before and after. 136 lines, 27 of them naming a server or a proxy, identical.
The equivalence test checks every tally against listing that resource and counting it, and every server and proxy row against the full record field by field.
It is guarded by a check that some server actually reported a non-zero count. That guard caught the first version of this test, where the records were being reported through the wrong field and every counter was zero -- the test would otherwise have passed by comparing zeros.
Both behaviours were also checked against mutations: counting frozen resources as normal, and forcing the counts to be taken the old way. Each fails.
Measurements were taken with the arms interleaved and the changed arm run twice.
Relationship to the namespace listing
This removes the scrape's call to
list_namespacesentirely. That listing counts each namespace's tables, which is fixed separately in #523 -- still worth having, because the/namespacesroute does read those counts. The scrape never did.Suites: 327 metadata tests, 47 metaserver binary tests, 246 consensus tests, and
cargo check --all-targetsclean.