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

Make KVTree file writes failure resistant #40

Open
tonyhutter opened this issue Sep 29, 2020 · 3 comments
Open

Make KVTree file writes failure resistant #40

tonyhutter opened this issue Sep 29, 2020 · 3 comments

Comments

@tonyhutter
Copy link
Contributor

If there's a crash during kvtree_file_write(), it could result in a corrupted kvtree file (ECP-VeloC/AXL#83). @adammoody suggested that each kvtree_file_write() create a new file that gets rename()'d to the final filename. A rename() is atomic on Linux. This means that if there's a failure during the write, then kvtree_read_file() is going to return the kvtree state before the failed write rather than an error. We also may want to consider writing to the new kvtree file with O_DIRECT or O_ATOMIC for even more protection.

@tonyhutter
Copy link
Contributor Author

We'll also need to consider what to do about kvtree_write_fd(), since we export that to the user.

@adammoody
Copy link
Contributor

For kvtree_write_fd, let's leave file consistency issues as the responsibility of the caller, since the caller is the one that opened the file in this case.

@adammoody
Copy link
Contributor

adammoody commented Oct 2, 2020

So that I don't forget, here are some other things that come to mind when we get around to this. There are at least two types of write calls we'll want to change to write to a temp file and then rename instead of writing directly to the real file.

The first is kvtree_write_file, which should be straight-forward:

int kvtree_write_file(const char* file, const kvtree* hash)

The second kind of write is kvtree_write_close_unlock, which is paired with kvtree_lock_open_read to do locked read-modify-write operations on a file:

int kvtree_write_close_unlock(const char* file, int* fd, const kvtree* hash)

For this second case, we may need both the open and close functions to deal with the temp name. Also, we'll need to figure out how to do the locking correctly. The locking is needed because there are two different processes trying to read/write the same file, and the locking is needed to serialize access. I don't know off hand if we need to lock the source file name, the new temp file name, or both.

Finally, we need to consider how to delete a kvtree file. Currently, users of kvtree may delete the kvtree file directly when they cleanup. However, we'll now need to delete both the file and its temp name in case a process died before it could rename the temp file. For that, we'll either want add a parameter to the interface so that the user of kvtree can provide the name of the temp file, so they know what to delete, or we'll need to define a kvtree_unlink_file function that deletes both files and then change any user code that is currently directly deleting the kvtree files to call this new function instead.

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