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

expose doc store cache size #1403

Merged
merged 2 commits into from Jul 5, 2022
Merged

expose doc store cache size #1403

merged 2 commits into from Jul 5, 2022

Conversation

PSeitz
Copy link
Contributor

@PSeitz PSeitz commented Jul 1, 2022

expose lru doc store cache size
optimize doc store cache size

@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2022

Codecov Report

Merging #1403 (5750224) into main (2ed5cc8) will decrease coverage by 0.05%.
The diff coverage is 57.14%.

@@            Coverage Diff             @@
##             main    #1403      +/-   ##
==========================================
- Coverage   94.30%   94.25%   -0.06%     
==========================================
  Files         236      236              
  Lines       43651    43680      +29     
==========================================
+ Hits        41165    41170       +5     
- Misses       2486     2510      +24     
Impacted Files Coverage Δ
src/functional_test.rs 0.00% <0.00%> (ø)
src/indexer/segment_writer.rs 96.40% <ø> (ø)
src/store/reader.rs 82.26% <16.66%> (-4.94%) ⬇️
src/core/searcher.rs 83.06% <20.00%> (-5.64%) ⬇️
src/core/segment_reader.rs 91.16% <100.00%> (ø)
src/indexer/index_writer.rs 96.82% <100.00%> (+<0.01%) ⬆️
src/indexer/merger.rs 98.97% <100.00%> (ø)
src/reader/mod.rs 94.26% <100.00%> (+0.14%) ⬆️
src/store/mod.rs 99.17% <100.00%> (ø)
src/fastfield/reader.rs 89.07% <0.00%> (-0.85%) ⬇️

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 2ed5cc8...5750224. Read the comment docs.

@PSeitz PSeitz requested a review from fulmicoton July 1, 2022 06:53
src/core/searcher.rs Outdated Show resolved Hide resolved
src/store/reader.rs Outdated Show resolved Hide resolved
@fulmicoton
Copy link
Collaborator

that PR is allowing to set the cache size on the docstore. The PR does not really explain why.
My understanding is that the benefit is to allow a cache of size 1 when merging?

Could this be done on segment reader creation?
Or not done at all actually? Are you thinking of enabling a large LRU size?

expose lru doc store cache size
optimize doc store cache size
@PSeitz
Copy link
Contributor Author

PSeitz commented Jul 4, 2022

that PR is allowing to set the cache size on the docstore. The PR does not really explain why. My understanding is that the benefit is to allow a cache of size 1 when merging?

Could this be done on segment reader creation? Or not done at all actually? Are you thinking of enabling a large LRU size?

One benefit is to reduce the cache size for merging. The other benefit is to reduce memory consumption for quickwit fetch_docs. The cache size is much more important with the larger blocks now.

@PSeitz PSeitz merged commit 2c17271 into main Jul 5, 2022
@PSeitz PSeitz deleted the docstore_cache_size branch July 5, 2022 02:28
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.

None yet

3 participants