diff --git a/config b/config index a0922b36..1cdfb75a 100644 --- a/config +++ b/config @@ -103,6 +103,11 @@ # Folder for storing local collections, created if not present #filesystem_folder = ~/.config/radicale/collections +# Sync all changes to disk during requests. (This can impair performance.) +# Disabling it increases the risk of data loss, when the system crashes or +# power fails! +#fsync = True + # Command that is run after changes to storage #hook = # Example: git add -A && (git diff --cached --quiet || git commit -m "Changes by "%(user)s) diff --git a/radicale/config.py b/radicale/config.py index 980cf78c..1e02656d 100644 --- a/radicale/config.py +++ b/radicale/config.py @@ -58,6 +58,7 @@ INITIAL_CONFIG = { "type": "multifilesystem", "filesystem_folder": os.path.expanduser( "~/.config/radicale/collections"), + "fsync": "True", "hook": ""}, "logging": { "config": "/etc/radicale/logging", diff --git a/radicale/storage.py b/radicale/storage.py index 2af4bac7..db68d42c 100644 --- a/radicale/storage.py +++ b/radicale/storage.py @@ -37,9 +37,8 @@ from hashlib import md5 from importlib import import_module from itertools import groupby from random import getrandbits -from tempfile import TemporaryDirectory +from tempfile import TemporaryDirectory, NamedTemporaryFile -from atomicwrites import AtomicWriter import vobject @@ -168,23 +167,6 @@ def path_to_filesystem(root, *paths): return safe_path -def sync_directory(path): - """Sync directory to disk. - - This only works on POSIX and does nothing on other systems. - - """ - if os.name == "posix": - fd = os.open(path, 0) - try: - if hasattr(fcntl, "F_FULLFSYNC"): - fcntl.fcntl(fd, fcntl.F_FULLFSYNC) - else: - os.fsync(fd) - finally: - os.close(fd) - - class UnsafePathError(ValueError): def __init__(self, path): message = "Can't translate name safely to filesystem: %s" % path @@ -209,16 +191,6 @@ class EtagMismatchError(ValueError): super().__init__(message) -class _EncodedAtomicWriter(AtomicWriter): - def __init__(self, path, encoding, mode="w", overwrite=True): - self._encoding = encoding - return super().__init__(path, mode, overwrite=True) - - def get_fileobject(self, **kwargs): - return super().get_fileobject( - encoding=self._encoding, prefix=".Radicale.tmp-", **kwargs) - - class Item: def __init__(self, collection, item, href, last_modified=None): self.collection = collection @@ -420,8 +392,23 @@ class Collection(BaseCollection): @contextmanager def _atomic_write(self, path, mode="w"): - with _EncodedAtomicWriter(path, self.encoding, mode).open() as fd: - yield fd + dir = os.path.dirname(path) + tmp = NamedTemporaryFile(mode=mode, dir=dir, encoding=self.encoding, + delete=False, prefix=".Radicale.tmp-") + try: + yield tmp + if self.configuration.getboolean("storage", "fsync"): + if os.name == "posix" and hasattr(fcntl, "F_FULLFSYNC"): + fcntl.fcntl(tmp.fileno(), fcntl.F_FULLFSYNC) + else: + os.fsync(tmp.fileno()) + tmp.close() + os.rename(tmp.name, path) + except: + tmp.close() + os.remove(tmp.name) + raise + self._sync_directory(dir) def _find_available_file_name(self): # Prevent infinite loop @@ -431,6 +418,25 @@ class Collection(BaseCollection): return file_name raise FileExistsError(errno.EEXIST, "No usable file name found") + @classmethod + def _sync_directory(cls, path): + """Sync directory to disk. + + This only works on POSIX and does nothing on other systems. + + """ + if not cls.configuration.getboolean("storage", "fsync"): + return + if os.name == "posix": + fd = os.open(path, 0) + try: + if hasattr(fcntl, "F_FULLFSYNC"): + fcntl.fcntl(fd, fcntl.F_FULLFSYNC) + else: + os.fsync(fd) + finally: + os.close(fd) + @classmethod def _makedirs_synced(cls, filesystem_path): """Recursively create a directory and its parents in a sync'ed way. @@ -447,7 +453,7 @@ class Collection(BaseCollection): cls._makedirs_synced(parent_filesystem_path) # Possible race! os.makedirs(filesystem_path, exist_ok=True) - sync_directory(parent_filesystem_path) + cls._sync_directory(parent_filesystem_path) @classmethod def discover(cls, path, depth="0"): @@ -561,7 +567,7 @@ class Collection(BaseCollection): if os.path.exists(filesystem_path): os.rename(filesystem_path, os.path.join(tmp_dir, "delete")) os.rename(tmp_filesystem_path, filesystem_path) - sync_directory(parent_dir) + cls._sync_directory(parent_dir) return cls(sane_path, principal=principal) @@ -570,9 +576,9 @@ class Collection(BaseCollection): os.rename( path_to_filesystem(item.collection._filesystem_path, item.href), path_to_filesystem(to_collection._filesystem_path, to_href)) - sync_directory(to_collection._filesystem_path) + cls._sync_directory(to_collection._filesystem_path) if item.collection._filesystem_path != to_collection._filesystem_path: - sync_directory(item.collection._filesystem_path) + cls._sync_directory(item.collection._filesystem_path) def list(self): try: @@ -648,9 +654,9 @@ class Collection(BaseCollection): dir=parent_dir) as tmp_dir: os.rename(self._filesystem_path, os.path.join( tmp_dir, os.path.basename(self._filesystem_path))) - sync_directory(parent_dir) + self._sync_directory(parent_dir) else: - sync_directory(parent_dir) + self._sync_directory(parent_dir) else: # Delete an item if not is_safe_filesystem_path_component(href): @@ -663,7 +669,7 @@ class Collection(BaseCollection): if etag and etag != get_etag(text): raise EtagMismatchError(etag, get_etag(text)) os.remove(path) - sync_directory(os.path.dirname(path)) + self._sync_directory(os.path.dirname(path)) def get_meta(self, key): if os.path.exists(self._props_path): diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py index ad061ef1..acb06ff8 100644 --- a/radicale/tests/test_base.py +++ b/radicale/tests/test_base.py @@ -726,6 +726,8 @@ class TestMultiFileSystem(BaseRequests, BaseTest): super().setup() self.colpath = tempfile.mkdtemp() self.configuration.set("storage", "filesystem_folder", self.colpath) + # Disable syncing to disk for better performance + self.configuration.set("storage", "fsync", "False") self.application = Application(self.configuration, self.logger) def teardown(self): @@ -740,6 +742,8 @@ class TestCustomStorageSystem(BaseRequests, BaseTest): super().setup() self.colpath = tempfile.mkdtemp() self.configuration.set("storage", "filesystem_folder", self.colpath) + # Disable syncing to disk for better performance + self.configuration.set("storage", "fsync", "False") self.application = Application(self.configuration, self.logger) def teardown(self): diff --git a/setup.py b/setup.py index 839f982d..156afd7d 100755 --- a/setup.py +++ b/setup.py @@ -66,7 +66,7 @@ setup( packages=["radicale"], provides=["radicale"], scripts=["bin/radicale"], - install_requires=["vobject", "atomicwrites"], + install_requires=["vobject"], setup_requires=pytest_runner, tests_require=[ "pytest-runner", "pytest-cov", "pytest-flake8", "pytest-isort"],