Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set create file option to false on opening a segment file just created #56

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

ylgrgyq
Copy link
Contributor

@ylgrgyq ylgrgyq commented Sep 9, 2023

Hi there,

I think maybe we can set the create option in std::fs::OpenOptions to false to open a Segment file just created

@timvisee
Copy link
Member

Thank you for diving into this.

Looks correct to me, though I'll ask two others to review this as well.

@agourlay
Copy link
Member

There is a Clippy lint to fix on master to be able to move forward here.

warning: usage of an `Arc` that is not `Send` or `Sync`
   --> src/mmap_view_sync.rs:150:20
    |
150 |             inner: Arc::new(UnsafeCell::new(mmap)),
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: the trait `Sync` is not implemented for `UnsafeCell<MmapMut>`
    = note: required for `Arc<UnsafeCell<MmapMut>>` to implement `Send` and `Sync`
    = help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
    = note: `#[warn(clippy::arc_with_non_send_sync)]` on by default

@timvisee timvisee force-pushed the fix/open-segment-file-after-create branch from b2b4b0f to d0946ec Compare September 26, 2023 06:42
@timvisee
Copy link
Member

Thanks for this change @ylgrgyq!

@timvisee timvisee merged commit f22abd5 into qdrant:master Sep 26, 2023
@ylgrgyq ylgrgyq deleted the fix/open-segment-file-after-create branch October 10, 2023 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants