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

disregarding "db/tx.17a6f6aeb948b94b", it is zero length #33

Open
ettom opened this issue Jan 3, 2024 · 6 comments
Open

disregarding "db/tx.17a6f6aeb948b94b", it is zero length #33

ettom opened this issue Jan 3, 2024 · 6 comments

Comments

@ettom
Copy link
Contributor

ettom commented Jan 3, 2024

Occasionally, if a writer is cancelled, a 0-length file is left behind. This can be reproduced by running

for i in $(seq 0 100000); do echo "test $i $i" | timeout 0.01s sonnerie -d db add --format "u"; done

The timeout will probably need adjusting based on the speed of the filesystem, but on my machine this very quickly produces lots of empty transaction files.

I suppose the behaviour is to be expected and sonnerie can't really prevent this from happening (?), but from an UX perspective, perhaps compacting could also clean up these empty files? Maybe an option flag to do it automatically?

@njaard
Copy link
Owner

njaard commented Jan 4, 2024

The reason there are empty files is that a filename needs to be reserved before the committed transaction file can atomically replace it. That prevents another writer getting the same filename which you then replace, deleting the other transaction.

An alternative to creating the "reservation files" is to use the option RENAME_NOREPLACE of the system call renameat2. The current strategy can be used in situations where RENAME_NOREPLACE isn't available.

@ettom
Copy link
Contributor Author

ettom commented Feb 8, 2024

What do you recommend as a solution for now? Is it safe to delete 0-length files before compacting?

@njaard
Copy link
Owner

njaard commented Feb 8, 2024

Empty files can be deleted. Empty main files cannot be deleted because they are what sonnerie uses to identify a database.

@njaard
Copy link
Owner

njaard commented Feb 10, 2024

@ettom I have committed the change that removes those eprintlns from the library and into the CLI.

I will leave this ticket open to track that those empty files are created in the first place. That would be an API-breaking change because commit_to assumes it's allowed to overwrite existing files, but in order to avoid creating empty files, it would be forbidden to do that. However, compactions replace one of the files they compact, either main or the last transaction file they replaced .

@ettom
Copy link
Contributor Author

ettom commented Feb 11, 2024

Thanks, sounds great! Would you mind creating a release?

@njaard
Copy link
Owner

njaard commented Feb 11, 2024

released!

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

No branches or pull requests

2 participants