Automatically opt-out of multi-mmap support by testing support#7618
Automatically opt-out of multi-mmap support by testing support#7618
Conversation
|
I did some manual testing with Docker/WSL on Windows. Good news, these changes work! Without this patchOn Windows, I start a Qdrant container like this with a bind mount: docker run --rm -it -p 6333:6333 -p 6334:6334 -v /c/Users/timvisee/qdrant/storage:/qdrant/storage:z qdrant/qdrantThen I upsert points: docker run -it --rm --net=host qdrant/bfb:dev ./bfb --skip-wait-index --indexing-threshold 0After I restart the container, vector data is gone an reports all zeroes: 
With this patchWhen using this patch, we now see a warning printed in the logs (4th line) telling us multi-mmap is disabled on this environment: 
Then, after upserting again, vector data survives any number of container restarts! 
|
e32b48c to
7cc7405
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| }); | ||
|
|
||
| /// If multi-mmap support is checked at Qdrant startup, the result is stored in this cell. | ||
| pub static MULTI_MMAP_SUPPORT_CHECK_RESULT: OnceLock<bool> = OnceLock::new(); |
There was a problem hiding this comment.
Feel free to disregard this stylistic comment.
This intermediate global variable should be possible to avoid by wrapping the above logic in a function, and use it to set MULTI_MMAP_IS_SUPPORTED directly after doing the check in main.rs
There was a problem hiding this comment.
I don't fully get what you mean. Mind to open a PR on top on dev as proposal?
| } | ||
|
|
||
| // Opt-out if environment variable is set | ||
| if supported && std::env::var_os("QDRANT_NO_MULTI_MMAP").is_some_and(|val| !val.is_empty()) { |
There was a problem hiding this comment.
Can this be a config option rather than an env var?
There was a problem hiding this comment.
We can, though it'd be a bit harder to propagate.
Since I deem this an experimental feature (which we don't know enough about yet), I vote to not put it in settings and keep it as undocumented variable.
Once we know more about this, both in terms of behavior and uses cases, we can still promote it to proper setting.
What do you think?
I'll merge this one now. But if you'd still prefer a setting, we can always do that in a follow-up PR.
Depends on #7614
Fixes #7610, #6676
Automatically opt-out of using multiple memory maps on the same file if the runtime support check for it fails at startup. This way users don't have to specify
QDRANT_NO_MULTI_MMAPand it should work out of the box.We already had this check in place to log a warning on environments that suffer from this problem.
I am definitely not proud of how the result of the check ends up in
MULTI_MMAP_IS_SUPPORTED. Please let me know if you have a better suggestion for it.When starting on an unsupported environment, users will see this warning in the logs:
Tasks
All Submissions:
devbranch. Did you create your branch fromdev?New Feature Submissions:
cargo +nightly fmt --allcommand prior to submission?cargo clippy --all --all-featurescommand?