Don't use HDF5's default locking behavior when opening files read only#38
Don't use HDF5's default locking behavior when opening files read only#38
Conversation
f6d235c to
461ad4c
Compare
* HDF5 uses file locks to implement a `Single Writer, Multiple Reader` model * We expect the SONATA HDF5 files to always be finished reading, so we don't benefit from this, and on some files systems, there are a finite amount of locks (plus it's a pessimizing default) * we don't want to rely on `HDF5_USE_FILE_LOCKING` to be set * see more info: https://support.hdfgroup.org/documentation/hdf5/latest/_file_lock.html * Note: this requires that any other files opened with the HDF5 library be opened with the same flags. Thus, if `h5py` is used to open the file concurrently with `libsonata`, there may be failures if the locking flags aren't set the same we are using `use_file_locking=True` and `ignore_when_disabled=True`
461ad4c to
8b25884
Compare
WeinaJi
left a comment
There was a problem hiding this comment.
I am not famliar with HighFive. This change looks correct according to the HDF5 doc.
I don't know if it is easy to test this change, but since the CI tests pass, I am ok with merging it.
|
I wonder if this should not be implemented in the HighFive library? If I understand correctly, the purpose of using HighFive is to make H5 file access easier, but here we're introducing a direct interaction with the HDF5 library? |
| , _ignore_when_disabled(ignore_when_disabled) { } | ||
|
|
||
| void apply(const hid_t list) const { | ||
| herr_t err = H5Pset_file_locking(list, _use_file_locking, _ignore_when_disabled); |
There was a problem hiding this comment.
From the documentation, it seems that this is supported from version 1.10.1, but if compiled with an older version this will crash? Shouldn't it be protected, or the HDF5 dependency set to a minimum version?
|
Also, another thought: could the file locking be disabled at compile time (when we install HDF5) to avoid misbehavior if using I understand this is meant to be used in a very specific, controlled environment, so would that be enough? |
It wouldn't compile, but I also don't think it's worth dealing with something that was first released in 2009.
I have a PR open already: highfive-devs/highfive#114
One can open the file in h5py with locking disabled, so I'm not sure if it's good to disable it at compile time. |
Single Writer, Multiple Readermodeldon't benefit from this, and on some files systems, there are a finite
amount of locks (plus it's a pessimizing default)
HDF5_USE_FILE_LOCKINGto be setbe opened with the same flags. Thus, if
h5pyis used to open the fileconcurrently with
libsonata, there may be failures if the lockingflags aren't set the same we are using
use_file_locking=Trueandignore_when_disabled=True