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

Use unsafe_cleanup!/1 to delete data after consumer has stopped #2271

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

magnetised
Copy link
Contributor

@magnetised magnetised commented Jan 30, 2025

When cleaning up a shape we call cleanup! and (in the case of FileStorage) attempt to remove the data directory while the CubDb process is still running.

This fails (File.rm_rf/1 returns {:error, :eexist, _}) because the files are still in use.

Instead of removing the directory on cleanup! while the process(es) are still running, instead teardown the consumer supervision tree and use unsafe_cleanup!/1 do remove the directories.

unsafe_cleanup! is already documented as something that should be runnable without any active processes, so this is an ideal use.

Fixes problems with https://github.com/electric-sql/stratovolt/pull/243

@magnetised magnetised force-pushed the magnetised/safe-shape-cleanup branch from 846d675 to c52b5d4 Compare January 30, 2025 13:42
@magnetised magnetised marked this pull request as ready for review January 30, 2025 14:01
@magnetised magnetised requested a review from msfstef January 30, 2025 14:02
Copy link
Contributor

@msfstef msfstef left a comment

Choose a reason for hiding this comment

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

Nice! Fixes a sentry error I wanted to chase up on as well, two birds with one stone - more unsafe cleanups is the way!

@magnetised magnetised merged commit 7f36cc1 into main Jan 30, 2025
33 checks passed
@magnetised magnetised deleted the magnetised/safe-shape-cleanup branch January 30, 2025 15:22
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