Skip to content

manifest: move L0-specific fields from TableMetadata into l0Sublevels#5857

Merged
RaduBerinde merged 1 commit intocockroachdb:masterfrom
RaduBerinde:l0-sublevels-work
Apr 7, 2026
Merged

manifest: move L0-specific fields from TableMetadata into l0Sublevels#5857
RaduBerinde merged 1 commit intocockroachdb:masterfrom
RaduBerinde:l0-sublevels-work

Conversation

@RaduBerinde
Copy link
Copy Markdown
Member

TableMetadata contained four L0-specific fields (SubLevel, L0Index,
minIntervalIndex, maxIntervalIndex) that were only meaningful for L0
files, only valid for the most recent Version's l0Sublevels, and written
exclusively by l0Sublevels code. Storing them on the shared TableMetadata
struct was fragile because any code could accidentally read stale values
from a non-current Version.

This change introduces an l0FileState struct and stores per-file state in
l0Sublevels via a fileState slice (indexed by file position) and a
fileStateMap (TableNum -> index). The L0Index concept is now implicit as
the index into fileState.

L0CompactionFiles.FilesIncluded is changed from bitSet (indexed by
L0Index) to map[base.TableNum]struct{}, eliminating the need for
L0Index to be accessible outside l0Sublevels.

An L0Organizer.SubLevelOf method is added for external callers (e.g.
generateSublevelInfo in compaction_picker.go) that need to look up a
file's sublevel.

Co-Authored-By: roachdev-claude roachdev-claude-bot@cockroachlabs.com

@RaduBerinde RaduBerinde requested review from annrpom and xxmplus March 24, 2026 21:31
@RaduBerinde RaduBerinde requested a review from a team as a code owner March 24, 2026 21:31
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@RaduBerinde
Copy link
Copy Markdown
Member Author

I noticed this TODO and had Claude take a stab at it, I think it did a pretty good job.

Copy link
Copy Markdown
Contributor

@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.

cool!!
:lgtm:

@annrpom reviewed 8 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on RaduBerinde and xxmplus).


compaction_test.go line 2392 at r1 (raw file):

							return err.Error()
						}
						_, err = strconv.Atoi(data[3:])

should we be discarding this sublevelNum?


internal/manifest/l0_sublevels_test.go line 337 at r1 (raw file):

						sublevels.fileStateMap[f.TableNum] = idx
						idx++
						_ = sublevelIdx

nit: sublevelIdx doesn't seem to be needed

TableMetadata contained four L0-specific fields (SubLevel, L0Index,
minIntervalIndex, maxIntervalIndex) that were only meaningful for L0
files, only valid for the most recent Version's l0Sublevels, and written
exclusively by l0Sublevels code. Storing them on the shared TableMetadata
struct was fragile because any code could accidentally read stale values
from a non-current Version.

This change introduces an l0FileState struct and stores per-file state in
l0Sublevels via a fileState slice (indexed by file position) and a
fileStateMap (TableNum -> index). The L0Index concept is now implicit as
the index into fileState.

L0CompactionFiles.FilesIncluded is changed from bitSet (indexed by
L0Index) to map[base.TableNum]struct{}, eliminating the need for
L0Index to be accessible outside l0Sublevels.

An L0Organizer.SubLevelOf method is added for external callers (e.g.
generateSublevelInfo in compaction_picker.go) that need to look up a
file's sublevel.

Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Copy link
Copy Markdown
Member Author

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

TFTR!

@RaduBerinde made 2 comments and resolved 1 discussion.
Reviewable status: 3 of 8 files reviewed, 1 unresolved discussion (waiting on annrpom and xxmplus).


compaction_test.go line 2392 at r1 (raw file):

Previously, annrpom (annie pompa) wrote…

should we be discarding this sublevelNum?

Added a check that it matches the current (implicit) sublevel.

@RaduBerinde RaduBerinde merged commit cbec1ce into cockroachdb:master Apr 7, 2026
9 checks passed
@RaduBerinde RaduBerinde deleted the l0-sublevels-work branch April 7, 2026 17:32
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.

3 participants