Skip to content

DirFileSystem does not propagate transaction context to underlying filesystem #1823

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

Open
patrickwolf opened this issue Apr 23, 2025 · 0 comments · May be fixed by #1824
Open

DirFileSystem does not propagate transaction context to underlying filesystem #1823

patrickwolf opened this issue Apr 23, 2025 · 0 comments · May be fixed by #1824

Comments

@patrickwolf
Copy link

patrickwolf commented Apr 23, 2025

When wrapping a transactional file:// backend in DirFileSystem (e.g. filesystem("dir", …, fs=filesystem("file"))), entering with fs.transaction: on the dir:// wrapper sets only the wrapper’s _intrans flag. Because DirFileSystem never delegates its transaction context down to the wrapped LocalFileSystem, all writes (even via fs.open(..., "wb")) commit immediately rather than being deferred to temp files and renamed on commit.

Steps to Reproduce

Create a transactional file:// filesystem and wrap it in dir://:

import os, tempfile, fsspec

tmp = tempfile.mkdtemp()
base = fsspec.filesystem("file")  
fs   = fsspec.filesystem("dir", path=tmp, fs=base)

Enter a transaction on the dir:// wrapper and write a file:

with fs.transaction:
    with fs.open("data.txt", "wb") as f:
        f.write(b"hello")
    exists_inside = os.path.exists(os.path.join(tmp, "data.txt"))
    print("Exists inside transaction?", exists_inside)

Observe that exists_inside is True, even though no commit has occurred yet.

Expected Behavior

  • Inside the with fs.transaction: block, no file should appear on disk.
  • After exiting the block without errors, the file should be atomically renamed into place.
  • On error, no partial files should remain.

Actual Behavior

  • The file is created on disk immediately, inside the transaction block.
  • There is no deferral to a temp file, and no atomic rename on commit

Proposed Fix

Override DirFileSystem.transaction to delegate to self.fs.transaction, so that the wrapper and wrapped FS share the same transaction context and _intrans flag.

Minimal Repro Code

import os, tempfile, fsspec

tmp  = tempfile.mkdtemp()
base = fsspec.filesystem("file")
fs   = fsspec.filesystem("dir", path=tmp, fs=base)

print("Before transaction:", base._intrans, fs._intrans)

with fs.transaction:
    print("Inside transaction:", base._intrans, fs._intrans)
    with fs.open("data.txt", "wb") as f:
        f.write(b"hello")
    # Should be False, but is True
    print("Exists inside?", os.path.exists(os.path.join(tmp, "data.txt")))

print("Clean up")
os.remove(os.path.join(tmp, "data.txt"))
os.rmdir(tmp)

Output

Before transaction: False False
Inside transaction: False True
Exists inside? True
Clean up

Environment

fsspec version: 2025.3.2

patrickwolf added a commit to patrickwolf/filesystem_spec that referenced this issue Apr 23, 2025
DirFileSystem now delegates its `transaction` and `transaction_type` to the wrapped filesystem, ensuring that `with fs.transaction:` on a `dir://` instance correctly toggles the underlying `file://` transaction. This restores atomic write semantics when using the directory wrapper.

Closes fsspec#1823
patrickwolf added a commit to patrickwolf/filesystem_spec that referenced this issue Apr 24, 2025
This test verifies that the fix for issue fsspec#1823 works correctly by ensuring
that DirFileSystem properly propagates its transaction context to the underlying
filesystem. When a transaction is started on a DirFileSystem, both the wrapper
and the wrapped filesystem should have their _intrans flags set, allowing writes
to be deferred until the transaction is committed.

Test confirms that:
- Both filesystems have _intrans=True inside transaction
- Files do not appear on disk until transaction commits
- Files appear correctly after transaction commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant