src/geoipyupdate/__init__.py: try os.replace() before shutil.move()

It turns out that shutil.move() is not atomic, which can lead to
issues when replacing the database under heavy load. There is an
alternative in os.replace() that *is* atomic, but does not work across
filesystems. For the best of both worlds, we now try os.replace(), and
fall back to shutil.move() if it fails.

Testing shows that os.replace() raises an OSError across multiple
filesystems, so that's what we catch. The documentation doesn't
specify one way or the other.

Thanks to Paulo Magalhães for report.

diff --git a/src/geoipyupdate/__init__.py b/src/geoipyupdate/__init__.py
index ace4bdc0b16d3cd2cc164a6b9d4b6615323a69a4..a9e3bf5b0cc945c2c8e53fe28e2a7e34fd4a1743 100644 (file)
--- a/src/geoipyupdate/__init__.py
+++ b/src/geoipyupdate/__init__.py
@@ -131,5 +131,12 @@ def main():
         # minus whatever the user has set in his umask.
         os.chmod(g.name, 0o664 & ~old_umask)
 
-        # Overwrite the old database file with the new (gunzipped) one.
-        shutil.move(g.name, dbfile)
+        # Overwrite the old database file with the new (gunzipped)
+        # one. The os.replace() method is preferable because it is
+        # atomic, but it will raise an OSError if the source and
+        # destination are on different filesystems. We fall back
+        # to shutil.move in that case.
+        try:
+            os.replace(g.name, dbfile)
+        except OSError:
+            shutil.move(g.name, dbfile)