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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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

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

Fallback to fsync if fdatasync isn't available

swh/objstorage/backends/pathslicing.py
135

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

LGTM.

swh/objstorage/backends/pathslicing.py
135

*thumb up*

This revision is now accepted and ready to land.Jun 19 2019, 3:44 PM

Support old unittest.mock

This revision was automatically updated to reflect the committed changes.