Atomic object writes #157
Atomic object writes #157
Conversation
When Grit writes a loose object via the LooseStorage class,
it just opens the object file and starts writing. This works
most of the time, but can be a problem in some corner cases,
including:
1. If another process tries to write the same object
simultaneously, the writes may be interleaved and the
object can be corrupted.
2. If another process tries to read the object
simultaneously, it may see the object in a half-written
state.
3. If the process or machine crashes during the write, we
may leave a half-written corrupt object.
This can be solved by writing the object to a temporary file
and linking it into place. This is the same strategy used by
git itself.
When we write a loose object to disk, we simply close the file object before moving it into place. If the machine crashes shortly after our write, the contents may not have been committed to disk (depending your filesystem, usually the metadata is, and you end up with a corrupt, zero-length loose object file). This is especially bad because we report that the object is successfully written, which means we may have updated refs to point to it. A corrupt object at that point means not only does the operation fail, but the repository is left in a corrupted and unusable state. We can fix this by calling fsync on the object file before linking it into place. Between this and the previous commit, our object writing should now behave exactly like git's internal routines.
|
This looks great. |
|
Oops this is failing under Ruby 1.8.7 with: |
|
So 1.8.7 doesn't take the extra options argument. The mode default is:
The w+ mode is:
Seems like maybe we could drop the argument entirely and roll with that. The other huge issue here though is encodings. If we drop the |
|
Fixed this up in b49a6ff. Should be fine. |
|
Yeah, exactly, I was worried about losing the "b" flag. Your fix looks good. Thanks. |
We occasionally see corrupted empty loose object files on GitHub after a machine failover. The problem is that Grit is not as careful with its object writes as Git itself. This PR should fix that.
/cc @rtomayko