Skip to content

Conversation

@jbowens
Copy link
Collaborator

@jbowens jbowens commented Nov 5, 2025

A handful of small clean up items surrounding WALs and the flushable queue.

Document this subtlety that multiple flushable entries may share the same
logNum.
These MinRecycleLogNum and SetMinRecycleLogNum methods were only used to
ratchet the minimum log num, so this commit consolidates the two under a
RatchetMinRecycleLogNum method.
@jbowens jbowens requested a review from a team as a code owner November 5, 2025 21:56
@jbowens jbowens requested a review from xxmplus November 5, 2025 21:56
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Adapt TestVersionSetSeqNums to use pebble.Peek to identify the current active
MANIFEST. Previously this unit test would use the last MANIFEST file returned
by the List call.

Additionally, on failure this test now logs the filesystem operations executed
during the test.
Copy link
Member

@xinhaoz xinhaoz left a comment

Choose a reason for hiding this comment

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

:lgtm:

@xinhaoz reviewed 1 of 1 files at r1, 4 of 4 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @xxmplus)


wal/failover_manager.go line 529 at r2 (raw file):

	wm.recycler.Init(o.MaxNumRecyclableLogs)
	for _, ll := range initial {
		wm.recycler.RatchetMinRecycleLogNum(ll.Num + 1)

TIL the meaning of this word outside of slang.

Copy link
Collaborator Author

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

TFTR!

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @xinhaoz and @xxmplus)


wal/failover_manager.go line 529 at r2 (raw file):

Previously, xinhaoz (Xin Hao Zhang) wrote…

TIL the meaning of this word outside of slang.

😎

@jbowens jbowens merged commit f11535e into cockroachdb:master Nov 7, 2025
9 checks passed
@jbowens jbowens deleted the flushableCleanup branch November 7, 2025 19:16
Copy link
Member

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

:lgtm:

Reviewable status: all files reviewed, 1 unresolved discussion


wal/failover_manager.go line 529 at r2 (raw file):

Previously, jbowens (Jackson Owens) wrote…

😎

Main product photo
image.png

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