Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
17 changes: 11 additions & 6 deletions datalad_fuse/fuse_.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ def getattr(self, path, fh=None):
# We should just fabricate stats from the key here or not even
# bother???!
lgr.debug("File not already open")
fsspec_file = self._adapter.open(path)
with self.rwlock:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we move locking down into _adapter and make it repository specific since we have caches per repository?! then outside code would be able to parallelize across repositories.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@yarikoptic No, as we also need to lock whenever using a pseudo-filehandle returned from _adapter.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

then we might need a helper to return the appropriate (repository specific) lock for a given path to be used.

fsspec_file = self._adapter.open(path)
to_close = True
if fsspec_file:
if isinstance(fsspec_file, io.BufferedIOBase):
Expand All @@ -163,7 +164,8 @@ def getattr(self, path, fh=None):
fsspec_file, timestamp=self._adapter.get_commit_datetime(path)
)
if to_close:
fsspec_file.close()
with self.rwlock:
fsspec_file.close()
else:
# TODO: although seems to be logical -- seems to cause logging etc
# lgr.error("ENOENTing %s %s", path, fh)
Expand Down Expand Up @@ -200,7 +202,8 @@ def open(self, path, flags):
else:
# write/create
raise FuseOSError(EROFS)
fsspec_file = self._adapter.open(path)
with self.rwlock:
fsspec_file = self._adapter.open(path)
lgr.debug("Counter = %d", self._counter)
# TODO: threadlock ?
self._fhdict[self._counter] = fsspec_file # self.fs.open(fn, mode)
Expand All @@ -219,8 +222,9 @@ def read(self, _path, size, offset, fh):
# must be open already and we must have mapped it to fsspec file
# TODO: check for path to correspond?
f = self._fhdict[fh]
f.seek(offset)
return f.read(size)
with self.rwlock:
f.seek(offset)
return f.read(size)

def opendir(self, path):
lgr.debug("opendir(path=%r)", path)
Expand Down Expand Up @@ -259,7 +263,8 @@ def release(self, path, fh):
# files, so we need to provide some proper use of lru_cache
# to have not recently used closed
if f is not None and not f.closed:
f.close()
with self.rwlock:
f.close()
return 0

def readlink(self, path):
Expand Down
29 changes: 29 additions & 0 deletions datalad_fuse/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,26 @@

lgr = logging.getLogger("datalad.fuse.tests")

# We need large files that exceed the blocksize of 1 MiB
BIG_URLS = [
# (dataset path, URL, SHA256 digest)
(
"APL.pdf",
"http://www.softwarepreservation.org/projects/apl/Books/APROGRAMMING%20LANGUAGE",
"c65ccc2a97cdb6042641847112dc6e4d4d6e75fdedcd476fdd61f855711bbaf4",
),
(
"gameboy.pdf",
"https://archive.org/download/GameBoyProgManVer1.1/GameBoyProgManVer1.1.pdf",
"5263e6c1f5fa51fc6813d2ed71c738c887ec78554eef633ad72c6285f7ff9197",
),
(
"libpython3.10-stdlib_3.10.4-3_i386.deb",
"http://nyc3.clouds.archive.ubuntu.com/ubuntu/pool/main/p/python3.10/libpython3.10-stdlib_3.10.4-3_i386.deb",
"e79c1416ec792b61ad9770f855bf6889e57be5f6511ea814d81ef5f9b1a3eec9",
),
]


@pytest.fixture(autouse=True)
def capture_all_logs(caplog):
Expand Down Expand Up @@ -163,3 +183,12 @@ def superdataset(served_files, request, tmp_home, tmp_path_factory): # noqa: U1
sub = ds.create(dspath / "sub")
initdataset(sub, served_files, request.param == "remote")
return (ds, {os.path.join("sub", df.path): df.content for df in served_files})


@pytest.fixture
def big_url_dataset(tmp_home, tmp_path_factory): # noqa: U100
workpath = tmp_path_factory.mktemp("big_url_dataset")
ds = Dataset(workpath / "ds").create()
for path, url, _ in BIG_URLS:
ds.repo.add_url_to_file(path, url, options=["--relaxed"])
yield (ds, {path: digest for path, _, digest in BIG_URLS})
22 changes: 22 additions & 0 deletions datalad_fuse/tests/test_fuse.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from concurrent.futures import ThreadPoolExecutor, as_completed
from contextlib import contextmanager
import hashlib
import os.path
from pathlib import Path
import subprocess
Expand Down Expand Up @@ -231,3 +233,23 @@ def test_fuse_git_status(tmp_path):
universal_newlines=True,
)
assert r.stdout == ""


def test_parallel_access(tmp_path, big_url_dataset):
ds, data_files = big_url_dataset
with fusing(ds.path, tmp_path) as mount:
with ThreadPoolExecutor() as pool:
futures = {
pool.submit(sha256_file, mount / path): dgst
for path, dgst in data_files.items()
}
for fut in as_completed(futures.keys()):
assert fut.result() == futures[fut]


def sha256_file(path):
dgst = hashlib.sha256()
with open(path, "rb") as fp:
for chunk in iter(lambda: fp.read(65535), b""):
dgst.update(chunk)
return dgst.hexdigest()