Page MenuHomeSoftware Heritage

pathslicing: Make sure data is flushed to disk before renaming the tempfile
ClosedPublic

Authored by olasd on Jun 19 2019, 1:46 PM.

Diff Detail

Repository
rDOBJS Object storage
Branch
feature/fdatasync
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6348
Build 8800: tox-on-jenkinsJenkins
Build 8799: arc lint + arc unit

Event Timeline

olasd created this revision.Jun 19 2019, 1:46 PM
zack requested changes to this revision.Jun 19 2019, 2:44 PM
zack added inline comments.
swh/objstorage/backends/pathslicing.py
136

Is this because some python versions don't have it, or just for OS portability?

I'm asking because we now have evidence (in T1817) that fdatasync is critical for us, in the sense that we can lose data if it's not available. So just silently ignoring the fact it's not available makes me shiver :-)

I'm tempted to suggest failing to initialize if fdatasync is not available.

Either way, we should log an error message if it's not available.

This revision now requires changes to proceed.Jun 19 2019, 2:44 PM
olasd updated this revision to Diff 5363.Jun 19 2019, 3:38 PM

Fallback to fsync if fdatasync isn't available

swh/objstorage/backends/pathslicing.py
136

The new version falls back to fsync (available on all platforms) if fdatasync isn't available.

zack added a comment.Jun 19 2019, 3:44 PM

LGTM.

swh/objstorage/backends/pathslicing.py
136

*thumb up*

zack accepted this revision.Jun 19 2019, 3:44 PM
This revision is now accepted and ready to land.Jun 19 2019, 3:44 PM
olasd updated this revision to Diff 5365.Jun 19 2019, 4:06 PM

Support old unittest.mock

This revision was automatically updated to reflect the committed changes.