Skip to content

Fix transactions with compressed files - #2070

Open
Sanjays2402 wants to merge 2 commits into
fsspec:masterfrom
Sanjays2402:fix/transaction-compression
Open

Fix transactions with compressed files#2070
Sanjays2402 wants to merge 2 commits into
fsspec:masterfrom
Sanjays2402:fix/transaction-compression

Conversation

@Sanjays2402

Copy link
Copy Markdown
Contributor

Compressed transactions currently queue the compression wrapper, so transaction exit calls commit() on GzipFile and raises AttributeError. Queue the raw filesystem file before applying compression; the wrapper still flushes on context exit, then the raw file commits normally.

The regression test reproduces the reported local gzip text write and verifies that the committed file decompresses to the original data.

Fixes #1584

Compressed wrappers do not implement commit or discard, so tracking the
wrapper caused transaction exit to raise AttributeError. Track the raw
filesystem file before applying compression so it can be committed or
discarded after the wrapper flushes.

Add a local filesystem regression test for compressed text writes.

Fixes fsspec#1584
@martindurant

Copy link
Copy Markdown
Member

The windows failure looks genuine

A compression wrapper such as GzipFile does not close the underlying file
object it wraps, so on transaction commit the temp file could still hold
buffered bytes and a pending gzip trailer. On Linux/macOS the subsequent
rename happened to work and the data was flushed on interpreter cleanup,
but on Windows the move ran against a still-open handle and produced a
truncated archive (EOFError: Compressed file ended before the
end-of-stream marker was reached).

Close the temp file, if still open, before moving it into place.
@Sanjays2402

Copy link
Copy Markdown
Contributor Author

You were right, it was genuine. The Windows job failed in test_transaction_with_compression with EOFError: Compressed file ended before the end-of-stream marker was reached.

Cause: GzipFile does not close the file object it wraps, so at transaction commit LocalFileOpener.commit() was calling shutil.move() on a temp file that still had buffered bytes and the pending gzip trailer. On posix the rename works on the open handle and the flush lands anyway, so the test passed; on Windows the move happens against a live handle and you get a truncated archive.

Fix is one hunk in LocalFileOpener.commit(): close self.f if it is still open before moving the temp file into place.

Verified locally on macOS (test_local.py + test_memory.py green, transaction tests green) — I do not have a Windows box, so I am relying on the pytest-win job here for confirmation.

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 this pull request may close these issues.

Transaction doesn't work well with compression

2 participants