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

wal: delete recovered logs in Open #3338

Closed
wants to merge 1 commit into from

Conversation

jbowens
Copy link
Collaborator

@jbowens jbowens commented Feb 22, 2024

Adjust the Open logic to delete logs discovered on Open without ever propagating them to the WAL manager. This has the advantage that each WAL manager queue only needs to maintain information about WALs of the type that it creates. For example, if WAL failover was previously enabled but is no longer, any logical WALs that are split across multiple physical files will be deleted by Open and never need to enter the standalone manager's log queue.

Adjust the Open logic to delete logs discovered on Open without ever
propagating them to the WAL manager. This has the advantage that each WAL
manager queue only needs to maintain information about WALs of the type that it
creates. For example, if WAL failover was previously enabled but is no longer,
any logical WALs that are split across multiple physical files will be deleted
by Open and never need to enter the standalone manager's log queue.
@jbowens jbowens requested review from sumeerbhola and a team February 22, 2024 22:32
@cockroach-teamcity
Copy link
Member

This change is Reviewable

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 8 of 8 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jbowens)


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

	}
	wm.recycler.Init(o.MaxNumRecyclableLogs)
	if wm.recycler.MinRecycleLogNum() <= minRecycleLogNum {

should this be <


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

	}
	m.recycler.Init(o.MaxNumRecyclableLogs)
	if m.recycler.MinRecycleLogNum() <= minRecycleLogNum {

should this be <

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.

This approach was a bit fraught. It's difficult to preserve determinism of file deletions if we introduce the EnqueueJob after replaying the WALs. I'm closing this in favor of #3341.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jbowens)

@jbowens jbowens closed this Feb 23, 2024
@jbowens jbowens deleted the wal-open-refac branch February 23, 2024 16:14
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