Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release-23.1: sql: extend SHOW RANGES to include object size estimates #105306

Closed

Conversation

zachlite
Copy link
Contributor

Backport 1/1 commits from #103128.

/cc @cockroachdb/release

Release justification: Important observability feature


This commit adds object size estimates to SHOW RANGES WITH DETAILS.
A new override of crdb_internal.tenant_span_stats is introduced
to leverage batched span statistics requests to KV, so only 1 fan-out is
required.

Resolves: #97858
Epic: https://cockroachlabs.atlassian.net/browse/CRDB-24928

Release note (sql change): The SHOW RANGES command will now
emit span statistics when the DETAILS option is specified.
The statistics are included in a new column named 'span_stats',
as a JSON object.

The statistics are calculated for the identifier of each row:
SHOW RANGES WITH DETAILS will compute span statistics for each range.
SHOW RANGES WITH TABLES, DETAILS will compute span statistics
for each table, etc.

The 'span_stats' JSON object has the following keys:

  • approximate_disk_bytes
  • [key|val|sys|live|intent]_count
  • [key|val|sys|live|intent]_bytes

'approximate_disk_bytes' is an approximation of the total on-disk size
of the given object.

'key_count' is the number of meta keys tracked under key_bytes.

'key_bytes' is the number of bytes stored in all non-system
point keys, including live, meta, old, and deleted keys.
Only meta keys really account for the "full" key; value
keys only for the timestamp suffix.

'val_count' is the number of meta values tracked under val_bytes.

'val_bytes' is the number of bytes in all non-system version
values, including meta values.

'sys_count' is the number of meta keys tracked under sys_bytes.

'sys_bytes' is the number of bytes stored in system-local kv-pairs.
This tracks the same quantity as (key_bytes + val_bytes), but
for system-local metadata keys (which aren't counted in either
key_bytes or val_bytes). Each of the keys falling into this group
is documented in keys/constants.go under the LocalPrefix constant
and is prefixed by either LocalRangeIDPrefix or LocalRangePrefix.

'live_count' is the number of meta keys tracked under live_bytes.

'live_bytes' is the number of bytes stored in keys and values which can
in principle be read by means of a Scan or Get in the far future,
including intents but not deletion tombstones (or their intents).
Note that the size of the meta kv pair (which could be explicit or implicit)
is included in this. Only the meta kv pair counts for the actual length
of the encoded key (regular pairs only count the timestamp suffix).

'intent_count' is the number of keys tracked under intent_bytes.
It is equal to the number of meta keys in the system with
a non-empty Transaction proto.

'intent_bytes' is the number of bytes in intent key-value
pairs (without their meta keys).

This commit adds object size estimates to `SHOW RANGES WITH DETAILS`.
A new override of `crdb_internal.tenant_span_stats` is introduced
to leverage batched span statistics requests to KV, so only 1 fan-out is
required.

Resolves: cockroachdb#97858
Epic: https://cockroachlabs.atlassian.net/browse/CRDB-24928

Release note (sql change): The SHOW RANGES command will now
emit span statistics when the DETAILS option is specified.
The statistics are included in a new column named 'span_stats',
as a JSON object.

The statistics are calculated for the identifier of each row:
`SHOW RANGES WITH DETAILS` will compute span statistics for each range.
`SHOW RANGES WITH TABLES, DETAILS` will compute span statistics
for each table, etc.

The 'span_stats' JSON object has the following keys:
- approximate_disk_bytes
- [key|val|sys|live|intent]_count
- [key|val|sys|live|intent]_bytes

'approximate_disk_bytes' is an approximation of the total on-disk size
of the given object.

'key_count' is the number of meta keys tracked under key_bytes.

'key_bytes' is the number of bytes stored in all non-system
point keys, including live, meta, old, and deleted keys.
Only meta keys really account for the "full" key; value
keys only for the timestamp suffix.

'val_count' is the number of meta values tracked under val_bytes.

'val_bytes' is the number of bytes in all non-system version
values, including meta values.

'sys_count' is the number of meta keys tracked under sys_bytes.

'sys_bytes' is the number of bytes stored in system-local kv-pairs.
This tracks the same quantity as (key_bytes + val_bytes), but
for system-local metadata keys (which aren't counted in either
key_bytes or val_bytes). Each of the keys falling into this group
is documented in keys/constants.go under the LocalPrefix constant
and is prefixed by either LocalRangeIDPrefix or LocalRangePrefix.

'live_count' is the number of meta keys tracked under live_bytes.

'live_bytes' is the number of bytes stored in keys and values which can
in principle be read by means of a Scan or Get in the far future,
including intents but not deletion tombstones (or their intents).
Note that the size of the meta kv pair (which could be explicit or implicit)
is included in this. Only the meta kv pair counts for the actual length
of the encoded key (regular pairs only count the timestamp suffix).

'intent_count' is the number of keys tracked under intent_bytes.
It is equal to the number of meta keys in the system with
a non-empty Transaction proto.

'intent_bytes' is the number of bytes in intent key-value
pairs (without their meta keys).
@zachlite zachlite requested review from a team as code owners June 21, 2023 20:03
@blathers-crl
Copy link

blathers-crl bot commented Jun 21, 2023

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@zachlite
Copy link
Contributor Author

@rafiss, sorry about that - I did a bad rebase and had to reopen the PR.
I updated the oid to 2414.

@zachlite zachlite requested a review from a team June 21, 2023 20:05
@erikgrinaker
Copy link
Contributor

This shouldn't be backported until we resolve #105274, since it breaks SHOW RANGES.

@rafiss rafiss requested review from knz and removed request for a team July 18, 2023 21:03
@knz
Copy link
Contributor

knz commented Jul 19, 2023

Beware that the problem in #105638 still exists. Is it clever to introduce this functionality in 23.1 if it's going to break for any customer with a sizeable cluster?

Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In an abundance of caution i'm going to retract my approval until we make progress on #105638.

@erikgrinaker
Copy link
Contributor

We'll also need to solve #106097.

@zachlite zachlite closed this Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants