Skip to content

Close the WAL when it is no longer in use - #481

Open
samliok wants to merge 4 commits into
mainfrom
fix/wal-close-leak
Open

Close the WAL when it is no longer in use#481
samliok wants to merge 4 commits into
mainfrom
fix/wal-close-leak

Conversation

@samliok

@samliok samliok commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Closes the wal when we stop the validator

@samliok
samliok marked this pull request as draft August 3, 2026 21:40
Instance never closed the GarbageCollectedWAL it holds: createEpochConfig
overwrote i.wal with a freshly created one on every epoch, and Stop left
the last one open. Each dropped WAL keeps its underlying files open, so
every epoch change leaked a set of file handles, and they were never
released on shutdown.

Close the previous WAL before replacing it and close the WAL in Stop.
@samliok
samliok force-pushed the fix/wal-close-leak branch from 2605cf9 to 1fd99c1 Compare August 3, 2026 21:42
Comment thread instance.go Outdated
Comment thread instance.go Outdated
@samliok
samliok marked this pull request as ready for review August 4, 2026 21:20
Comment thread instance.go
// Stop the epoch before doing anything else, so that we don't process any more messages while we are changing epochs.
i.stopValidator()

// Wipe out the WALs from the config so we won't try to load them again

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wait, doesn't that effectively make us load an empty WAL next invocation of createEpochConfig ?

@yacovm yacovm left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we have a problem here when we need to create the GC WAL a second time, don't we? The value passed from the config is stale, but we still need something.

I think we should just export .wals in GarbageCollectedWAL or alternatively, return the new WALs after GC:

GarbageCollect(retentionTerm uint64) ([]DeletableWAL ,error) 

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.

2 participants