Skip to content

Conversation

@annrpom
Copy link
Contributor

@annrpom annrpom commented Nov 3, 2025

Blob file rewrite compactions happen independent of LSM levels, and so don't
contribute to the per-level 'compacted bytes' total. This patch adds separate
metrics recording the bytes read and bytes written during these compactions.

Fixes: #5444

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@annrpom annrpom marked this pull request as ready for review November 3, 2025 16:44
@annrpom annrpom requested a review from a team as a code owner November 3, 2025 16:44
@annrpom annrpom requested a review from sumeerbhola November 3, 2025 16:44
Copy link
Collaborator

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

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

@sumeerbhola reviewed 8 of 9 files at r1.
Reviewable status: 8 of 9 files reviewed, 3 unresolved discussions


-- commits line 4 at r1:
independent of the level?


testdata/compaction/value_separation line 546 at r1 (raw file):

    est. debt |   in progress |  cancelled |   failed |    problem spans |       read |    written
--------------+---------------+------------+----------+------------------+------------+-----------
           0B |        0 (0B) |     0 (0B) |        0 |                0 |        83B |       150B

why is the read smaller than the written?


blob_rewrite.go line 251 at r1 (raw file):

		// rewrites don't contribute to per-level compacted bytes.
		// Count blob value blocks read from the blob file during the rewrite.
		bytesRead := c.internalIteratorStats.BlockReads[blockkind.BlobValue].BlockBytes

Is this why we bytes read could be smaller, because we don't count the various meta blocks or properties etc., while the write path does? Which is fine.

@annrpom annrpom force-pushed the metric-blob-rewrite branch from 7ad4823 to 0f5721c Compare November 4, 2025 15:54
Copy link
Contributor Author

@annrpom annrpom left a comment

Choose a reason for hiding this comment

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

Reviewable status: 8 of 9 files reviewed, 3 unresolved discussions (waiting on @sumeerbhola)


-- commits line 4 at r1:

Previously, sumeerbhola wrote…

independent of the level?

yes; corrected wording!


blob_rewrite.go line 251 at r1 (raw file):

Previously, sumeerbhola wrote…

Is this why we bytes read could be smaller, because we don't count the various meta blocks or properties etc., while the write path does? Which is fine.

yes - i'll add a comment for this as well

@annrpom annrpom requested a review from sumeerbhola November 4, 2025 15:54
Copy link
Collaborator

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

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

:lgtm:

@sumeerbhola reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @annrpom)

Blob file rewrite compactions happen independent of LSM levels, and so don't
contribute to the per-level 'compacted bytes' total. This patch adds separate
metrics recording the bytes read and bytes written during these compactions.

Fixes: cockroachdb#5444
@annrpom annrpom force-pushed the metric-blob-rewrite branch from 0f5721c to 10f71d3 Compare November 6, 2025 20:04
@annrpom
Copy link
Contributor Author

annrpom commented Nov 6, 2025

TFTR! ('-')7

@annrpom annrpom merged commit 9b01218 into cockroachdb:master Nov 6, 2025
9 checks passed
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.

db: add metric encapsulating blob file rewrite bytes read, written

3 participants