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

Use write_all when writing blob data to file system #587

Merged
merged 2 commits into from
Nov 4, 2023

Conversation

sandreae
Copy link
Member

@sandreae sandreae commented Nov 2, 2023

We should use write_all instead of write when writing blob data to the filesystem as the latter only represents a "single attempt" to write a buffer to a file, whereas the former "continuously calls [write] until there is no more data to be written" or an error occurs. In using the later we were sometimes silently failing to write all data to the file system.

Closes #586

📋 Checklist

  • Add tests that cover your changes
  • Add this PR to the Unreleased section in CHANGELOG.md
  • Link this PR to any issues it closes
  • New files contain a SPDX license header

@adzialocha
Copy link
Member

Really good to know ✨✊🏻

Copy link

codecov bot commented Nov 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4809fab) 92.67% compared to head (9ca7317) 92.68%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #587      +/-   ##
==========================================
+ Coverage   92.67%   92.68%   +0.01%     
==========================================
  Files         106      106              
  Lines       18384    18405      +21     
==========================================
+ Hits        17037    17059      +22     
+ Misses       1347     1346       -1     
Files Coverage Δ
aquadoggo/src/db/stores/document.rs 98.76% <100.00%> (-0.10%) ⬇️
aquadoggo/src/db/stores/operation.rs 89.74% <100.00%> (+0.11%) ⬆️
aquadoggo/src/materializer/tasks/blob.rs 87.09% <100.00%> (+2.84%) ⬆️
aquadoggo/src/materializer/tasks/reduce.rs 94.84% <100.00%> (-0.09%) ⬇️
aquadoggo/src/test_utils/node.rs 95.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sandreae sandreae merged commit 1a7cefc into main Nov 4, 2023
10 checks passed
@adzialocha adzialocha deleted the corrupted-blob-fix branch November 4, 2023 13:33
adzialocha added a commit that referenced this pull request Nov 15, 2023
* main:
  Use `write_all`  when writing blob data to file system (#587)
  Use `p2panda-rs` `0.8.0` (#585)
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.

Materialised blobs sometimes corrupted / missing bytes
2 participants