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

fix[next]: Guard diskcache creation by file lock #1745

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ repos:
- devtools==0.12.2
- diskcache==5.6.3
- factory-boy==3.3.1
- filelock==3.16.1
- frozendict==2.4.6
- gridtools-cpp==2.3.8
- importlib-resources==6.4.5
Expand Down
8 changes: 4 additions & 4 deletions constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ executing==2.1.0 # via devtools, stack-data
factory-boy==3.3.1 # via gt4py (pyproject.toml), pytest-factoryboy
faker==33.0.0 # via factory-boy
fastjsonschema==2.20.0 # via nbformat
filelock==3.16.1 # via tox, virtualenv
filelock==3.16.1 # via gt4py (pyproject.toml), tox, virtualenv
fonttools==4.55.0 # via matplotlib
fparser==0.1.4 # via dace
frozendict==2.4.6 # via gt4py (pyproject.toml)
Expand Down Expand Up @@ -113,8 +113,8 @@ psutil==6.1.0 # via -r requirements-dev.in, ipykernel, pytest-xdist
ptyprocess==0.7.0 # via pexpect
pure-eval==0.2.3 # via stack-data
pybind11==2.13.6 # via gt4py (pyproject.toml)
pydantic==2.9.2 # via bump-my-version, pydantic-settings
pydantic-core==2.23.4 # via pydantic
pydantic==2.10.0 # via bump-my-version, pydantic-settings
pydantic-core==2.27.0 # via pydantic
pydantic-settings==2.6.1 # via bump-my-version
pydot==3.0.2 # via tach
pygments==2.18.0 # via -r requirements-dev.in, devtools, ipython, nbmake, rich, sphinx
Expand Down Expand Up @@ -159,7 +159,7 @@ stack-data==0.6.3 # via ipython
stdlib-list==0.10.0 # via tach
sympy==1.13.3 # via dace
tabulate==0.9.0 # via gt4py (pyproject.toml)
tach==0.14.3 # via -r requirements-dev.in
tach==0.14.4 # via -r requirements-dev.in
tomli==2.1.0 ; python_version < "3.11" # via -r requirements-dev.in, black, build, coverage, jupytext, mypy, pip-tools, pyproject-api, pytest, setuptools-scm, tach, tox
tomli-w==1.0.0 # via tach
tomlkit==0.13.2 # via bump-my-version
Expand Down
1 change: 1 addition & 0 deletions min-extra-requirements-test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ deepdiff==5.6.0
devtools==0.6
diskcache==5.6.3
factory-boy==3.3.0
filelock==3.0.0
frozendict==2.3
gridtools-cpp==2.3.8
hypothesis==6.0.0
Expand Down
1 change: 1 addition & 0 deletions min-requirements-test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ deepdiff==5.6.0
devtools==0.6
diskcache==5.6.3
factory-boy==3.3.0
filelock==3.0.0
frozendict==2.3
gridtools-cpp==2.3.8
hypothesis==6.0.0
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ dependencies = [
'devtools>=0.6',
'diskcache>=5.6.3',
'factory-boy>=3.3.0',
'filelock>=3.0.0',
'frozendict>=2.3',
'gridtools-cpp>=2.3.8,==2.*',
"importlib-resources>=5.0;python_version<'3.9'",
Expand Down
8 changes: 4 additions & 4 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ executing==2.1.0 # via -c constraints.txt, devtools, stack-data
factory-boy==3.3.1 # via -c constraints.txt, gt4py (pyproject.toml), pytest-factoryboy
faker==33.0.0 # via -c constraints.txt, factory-boy
fastjsonschema==2.20.0 # via -c constraints.txt, nbformat
filelock==3.16.1 # via -c constraints.txt, tox, virtualenv
filelock==3.16.1 # via -c constraints.txt, gt4py (pyproject.toml), tox, virtualenv
fonttools==4.55.0 # via -c constraints.txt, matplotlib
fparser==0.1.4 # via -c constraints.txt, dace
frozendict==2.4.6 # via -c constraints.txt, gt4py (pyproject.toml)
Expand Down Expand Up @@ -113,8 +113,8 @@ psutil==6.1.0 # via -c constraints.txt, -r requirements-dev.in, ipyk
ptyprocess==0.7.0 # via -c constraints.txt, pexpect
pure-eval==0.2.3 # via -c constraints.txt, stack-data
pybind11==2.13.6 # via -c constraints.txt, gt4py (pyproject.toml)
pydantic==2.9.2 # via -c constraints.txt, bump-my-version, pydantic-settings
pydantic-core==2.23.4 # via -c constraints.txt, pydantic
pydantic==2.10.0 # via -c constraints.txt, bump-my-version, pydantic-settings
pydantic-core==2.27.0 # via -c constraints.txt, pydantic
pydantic-settings==2.6.1 # via -c constraints.txt, bump-my-version
pydot==3.0.2 # via -c constraints.txt, tach
pygments==2.18.0 # via -c constraints.txt, -r requirements-dev.in, devtools, ipython, nbmake, rich, sphinx
Expand Down Expand Up @@ -158,7 +158,7 @@ stack-data==0.6.3 # via -c constraints.txt, ipython
stdlib-list==0.10.0 # via -c constraints.txt, tach
sympy==1.13.3 # via -c constraints.txt, dace
tabulate==0.9.0 # via -c constraints.txt, gt4py (pyproject.toml)
tach==0.14.3 # via -c constraints.txt, -r requirements-dev.in
tach==0.14.4 # via -c constraints.txt, -r requirements-dev.in
tomli==2.1.0 ; python_version < "3.11" # via -c constraints.txt, -r requirements-dev.in, black, build, coverage, jupytext, mypy, pip-tools, pyproject-api, pytest, setuptools-scm, tach, tox
tomli-w==1.0.0 # via -c constraints.txt, tach
tomlkit==0.13.2 # via -c constraints.txt, bump-my-version
Expand Down
22 changes: 19 additions & 3 deletions src/gt4py/next/program_processors/runners/gtfn.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
# SPDX-License-Identifier: BSD-3-Clause

import functools
import pathlib
import tempfile
import warnings
from typing import Any, Optional

import diskcache
import factory
import filelock
import numpy.typing as npt

import gt4py._core.definitions as core_defs
Expand Down Expand Up @@ -141,11 +144,24 @@ def fingerprint_compilable_program(inp: stages.CompilableProgram) -> str:

class FileCache(diskcache.Cache):
"""
This class extends `diskcache.Cache` to ensure the cache is closed upon deletion,
i.e. it ensures that any resources associated with the cache are properly
released when the instance is garbage collected.
This class extends `diskcache.Cache` to ensure the cache is properly
- opened on a distributed file system by using a file lock. This guards the creating of the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is writing not a problem? Except that it's much harder to make all processes write at the same time and there less likely to trigger.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, writing to the cache is actually fine. SQLite which is used internally by diskcache.Cache handles concurrency very well in most cases, except for changing settings of the database itself, and that is the problem here. On creation of the diskcache.Cache object, some sql PRAGMA statements are executed and they lead to the observed issues. It is not really clear why, this should work, but it doesn't.

cache object, which has been reported to cause `sqlite3.OperationalError: database is locked`
errors and slow startup times when multiple processes access the cache concurrently.
- closed upon deletion, i.e. it ensures that any resources associated with the cache are
properly released when the instance is garbage collected.
"""

def __init__(self, directory: Optional[str | pathlib.Path] = None, **settings: Any) -> None:
if directory:
lock_dir = pathlib.Path(directory).parent
else:
lock_dir = pathlib.Path(tempfile.gettempdir())

lock = filelock.FileLock(lock_dir / "file_cache.lock")
with lock:
super().__init__(directory=directory, **settings)

def __del__(self) -> None:
self.close()

Expand Down
Loading