goraft / raft

UNMAINTAINED: A Go implementation of the Raft distributed consensus protocol.
MIT License
2.43k stars 480 forks source link

Windows: log compaction problems #219

Open philips opened 10 years ago

philips commented 10 years ago

https://github.com/coreos/etcd/issues/711

Looks like a straight forward fix.

MartyMacGyver commented 10 years ago

I stumbled upon bug 711 a little while ago myself and did a bit of digging. In short, this really ought to be fixed in Go itself, but failing that, the easy way is to simply do a create/delete with error checking (the extra space isn't a concern since a new compacted logfile is created before the old one is remove in the rename process, and a mutex already guards the compaction operation.)

If atomicity of the rename itself is a concern then it's more complicated... but other similar bugfixes suggest the proper way to handle this in Windows (e.g., https://bugs.launchpad.net/juju-core/+bug/1240927), particularly the use of MoveFileEx (http://msdn.microsoft.com/en-us/library/windows/desktop/aa365240(v=vs.85).aspx)

This was previously raised as a bug against Go itself (https://code.google.com/p/go/issues/detail?id=3366) but it was rejected on the grounds that atomicity may not guaranteed in Windows. There's a much more thorough discussion of this problem as it affects the Python core code at http://bugs.python.org/issue8828, including a successful resolution.

MartyMacGyver commented 10 years ago

Update: Since there will be no further movement on correcting os.Rename() in Go, I've opened a request for an os.Replace() function (https://code.google.com/p/go/issues/detail?id=8914).