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

db: fix WAL offset in replayWAL errors #3862

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

jbowens
Copy link
Collaborator

@jbowens jbowens commented Aug 19, 2024

If an error occurs while replaying the WALs during Open, the error is annotated with the log number and the offset within the log. Previously, the offset was always zero. This also caused the queued flushables to be created with a zero logSize. This didn't matter in practice because the queued flushables would be flushed before Open returned.

This commit adds a PreviousFilesBytes total to the wal.Offset type to describe the number of bytes replayed from previous WAL segement files.

@jbowens jbowens requested a review from sumeerbhola August 19, 2024 14:56
@jbowens jbowens requested a review from a team as a code owner August 19, 2024 14:56
@cockroach-teamcity
Copy link
Member

This change is Reviewable

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: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @jbowens and @sumeerbhola)


wal/wal.go line 437 at r1 (raw file):

// offset.
func (o Offset) String() string {
	return fmt.Sprintf("(%s: %d), %d from previous files", o.PhysicalFile, o.Physical, o.PreviousFilesBytes)

[nit] you can consider omitting the new part when PrevioousFilesBytes == 0

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:

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jbowens)

If an error occurs while replaying the WALs during Open, the error is annotated
with the log number and the offset within the log. Previously, the offset was
always zero. This also caused the queued flushables to be created with a zero
logSize. This didn't matter in practice because the queued flushables would be
flushed before Open returned.

This commit adds a PreviousFilesBytes total to the wal.Offset type to describe
the number of bytes replayed from previous WAL segement files.
@jbowens
Copy link
Collaborator Author

jbowens commented Aug 20, 2024

TFTRs!

@jbowens jbowens merged commit aa32f0b into cockroachdb:master Aug 20, 2024
11 checks passed
@jbowens jbowens deleted the wal-offset branch August 20, 2024 17:05
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