search

realm / realm-core

Core database component for the Realm Mobile Database SDKs
https://realm.io
Apache License 2.0
1.01k stars 157 forks source link

Make compact() atomic on Windows #4111

Open blagoev opened 3 years ago

blagoev commented 3 years ago

Currently we have this comment that compact is not atomic and safe on Windows https://github.com/realm/realm-core/blob/0a4c622924018b434af293f9ec2bb9c6b2edbc01/src/realm/db.hpp#L283-L284 After a short discussion we have decided that since compact() is already exposed in Realm JS on Windows https://github.com/realm/realm-js/blob/master/src/js_realm.hpp#L339 we could improve the compact operation and make it atomic by using MoveFileEx using MOVEFILE_REPLACE_EXISTING and possibly MOVEFILE_WRITE_THROUGH flags which should result in an atomic and safe move on NTFS on Windows

sync-by-unito[bot] commented 1 year ago

➤ On 2022-03-25, bmunkholm commented:

[~yavor.georgiev] Do you know if this has been done?

sync-by-unito[bot] commented 1 year ago

➤ On 2022-03-25, fealebenpae commented:

No, this hasn’t been addressed yet.

sync-by-unito[bot] commented 1 year ago

➤ On 2022-03-25, bmunkholm commented:

Oh , that's interesting as the comment has been removed now :-)

sync-by-unito[bot] commented 1 year ago

➤ On 2022-03-25, fealebenpae commented:

It’s just in a different place now: https://github.com/realm/realm-core/blob/798335cf1cc054ae7abbe1d2c7e1efd2d7e982a5/src/realm/db.cpp#L1389

cmelchior commented 1 year ago

This is coming up in the context of the Kotlin SDK. How tricky would this be to fix?

fealebenpae commented 1 year ago

It's somewhat tricky. There isn't a clear-cut API that promises atomicity on Windows without caveats, unless we want to start enforcing same-volume origin when renaming files, and even then there are other caveats from what I can tell. It's safer to use the shouldCompactOnLaunch configuration option in any case, I believe.