Merged
Conversation
timvisee
commented
Jul 31, 2025
Comment on lines
+171
to
+172
| // Explicitly flush so we don't lose dirty pages | ||
| self.flusher()()?; |
Member
Author
There was a problem hiding this comment.
- because all set functions require
&mutwe are guaranteed to have exclusive access here, we're therefore safe to flush - we now flush both flags and the status file, we probably only need to flush flags
Note that we should assume that we can flush this structure at any time. New writes to the bool/null index should not be written to this structure directly and should be deferred until we flush, that will be implemented in a separate PR.
Member
Author
|
Let's finalize and merge https://github.com/qdrant/qdrant/pull/6954/files first, and see what parts of this remain relevant. |
coszio
approved these changes
Aug 4, 2025
Contributor
coszio
left a comment
There was a problem hiding this comment.
I commited a change which flushes flags only before reopening the mmap
Merged
Member
Author
|
Note: we should still merge this to satisfy #6954 (comment) |
77dff49 to
c596a83
Compare
This comment was marked as resolved.
This comment was marked as resolved.
timvisee
added a commit
that referenced
this pull request
Aug 11, 2025
* Directly flush flags, don't use single-use flusher * Explicitly flush before resizing flags file, prevent losing dirty pages * Don't create now obsolete flusher when opening mmap * clippy * Move flush call * flush before resizing only --------- Co-authored-by: Luis Cossío <luis.cossio@outlook.com>
Merged
6 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix flushing behavior in the dynamic mmap flags structure, affecting the null and boolean indices.
This makes two changes:
All Submissions:
devbranch. Did you create your branch fromdev?New Feature Submissions:
cargo +nightly fmt --allcommand prior to submission?cargo clippy --all --all-featurescommand?