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

FileHandle: Change from boxed to Arc. #1415

Merged
merged 2 commits into from
Jul 21, 2022

Conversation

pier-oliviert
Copy link
Contributor

Changing from a Box to an Arc would
allow for a user of tantivy to manage file handles outside of tantivy
and be able to manage their life cycle.

I'm currently working with tantivy and one of the thing I wanted to do was to manage file access outside of tantivy in a way that file access is abstracted completely from tantivy. I got everything working on my end, but I needed to make this change to tantivy because I needed shared ownership of a given FileHandle.

I believe this change is safe to other user of tantivy, but I am new to both Tantivy & Rust, so maybe there's something else that needs to change to make this happen.

Changing from a Box<dyn FileHandle> to an Arc<dyn FileHandle> would
allow for a user of tantivy to manage file handles outside of tantivy
and be able to manage their life cycle.
@codecov-commenter
Copy link

Codecov Report

Merging #1415 (1bd507a) into main (23fe73a) will decrease coverage by 0.01%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##             main    #1415      +/-   ##
==========================================
- Coverage   94.22%   94.21%   -0.02%     
==========================================
  Files         236      236              
  Lines       43778    43770       -8     
==========================================
- Hits        41250    41238      -12     
- Misses       2528     2532       +4     
Impacted Files Coverage Δ
src/directory/directory.rs 90.14% <ø> (ø)
src/directory/managed_directory.rs 88.88% <0.00%> (ø)
src/directory/ram_directory.rs 88.81% <0.00%> (ø)
src/directory/file_slice.rs 75.79% <100.00%> (ø)
src/directory/footer.rs 93.25% <100.00%> (ø)
src/directory/mmap_directory.rs 91.08% <100.00%> (ø)
src/termdict/sstable_termdict/termdict.rs 74.82% <100.00%> (+0.35%) ⬆️
.../termdict/sstable_termdict/sstable/block_reader.rs 80.35% <0.00%> (-1.79%) ⬇️
bitpacker/src/bitpacker.rs 96.03% <0.00%> (-1.00%) ⬇️
src/query/boolean_query/boolean_weight.rs 93.93% <0.00%> (-0.64%) ⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23fe73a...1bd507a. Read the comment docs.

@fulmicoton
Copy link
Collaborator

@pier-oliviert Your formatting settings seems incorrect.

make fmt should run the right settings.

@pier-oliviert pier-oliviert force-pushed the file-handle-arc-argument branch from 1bd507a to 6ab249e Compare July 18, 2022 11:55
@pier-oliviert
Copy link
Contributor Author

@fulmicoton just force-pushed what I hope is the final fix for the linter. Let me know if you need anything else from me.

@fulmicoton fulmicoton merged commit 775e936 into quickwit-oss:main Jul 21, 2022
@pier-oliviert pier-oliviert deleted the file-handle-arc-argument branch July 21, 2022 11:33
This was referenced Jan 13, 2023
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