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

Ensure write failures are handled during conversion #42

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

erik
Copy link
Contributor

@erik erik commented Feb 3, 2023

Fixes #28.

There are several places where File.write() was called but the return value was ignored, meaning any failures would be silent.

Tested this locally by creating an artificially small filesystem and placing either the temp directory (via --tempdir) or output file in it to simulate disk full scenarios.

(below is for Mac OS, but there are equivalent options for Linux)

# Create a filesystem with only 1MB of space, mount it at ./tinyfs
hdiutil create -size 1m -fs 'Case-sensitive HFS+' -volname tinyfs tinyfs.dmg
hdiutil attach tinyfs.dmg -mountpoint tinyfs -nobrowse -readwrite

And some test cases, both of which complete successfully without this PR:

$ go run main.go convert ./zurich_switzerland.mbtiles output.pmtiles --tmpdir tinyfs/
2023/02/03 11:13:09 convert.go:258: Querying total tile count...
2023/02/03 11:13:09 convert.go:274: Pass 1: Assembling TileID set
 100% |███████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| (2134/2134, 994737 it/s)
2023/02/03 11:13:09 convert.go:304: Pass 2: writing tiles
   0% |                                                                                                                              | (0/2134, 0 it/hr) [0s:0s]
2023/02/03 11:13:09 main.go:153: Failed to convert /Users/erik/work/go-pmtiles/zurich_switzerland.mbtiles, Failed to write to tempfile: write /Users/erik/work/go-pmtiles/tinyfs/pmtiles1125616747: no space left on device
exit status 1

$ go run main.go convert ./zurich_switzerland.mbtiles tinyfs/output.pmtiles
2023/02/03 11:18:03 convert.go:262: Querying total tile count...
2023/02/03 11:18:03 convert.go:278: Pass 1: Assembling TileID set
 100% |███████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| (2134/2134, 802018 it/s)
2023/02/03 11:18:03 convert.go:308: Pass 2: writing tiles
 100% |████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| (2134/2134, 17424 it/s)
2023/02/03 11:18:03 convert.go:362: # of addressed tiles:  2134
2023/02/03 11:18:03 convert.go:363: # of tile entries (after RLE):  1846
2023/02/03 11:18:03 convert.go:364: # of tile contents:  1667
2023/02/03 11:18:03 convert.go:386: Total dir bytes:  4720
2023/02/03 11:18:03 convert.go:387: Average bytes per addressed tile: 2.21
2023/02/03 11:18:03 main.go:153: Failed to convert /Users/erik/work/go-pmtiles/zurich_switzerland.mbtiles, Failed to copy data to outfile, write /Users/erik/work/go-pmtiles/tinyfs/output.pmtiles: no space left on device

Also just a note, there are definitely other places where the error return value should probably be checked to avoid silent data corruption (for example), but I haven't done an exhaustive review.

@bdon bdon merged commit 2b28280 into protomaps:main Feb 6, 2023
@bdon
Copy link
Member

bdon commented Feb 6, 2023

Thanks for the patch!

@erik erik deleted the ep/check-err branch February 6, 2023 06:59
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.

convert does not fail when disk space runs out
2 participants