You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@lucene.apache.org by "jpountz (via GitHub)" <gi...@apache.org> on 2024/03/26 16:31:35 UTC

[PR] Use `IOContext#RANDOM` when appropriate. [lucene]

jpountz opened a new pull request, #13222:
URL: https://github.com/apache/lucene/pull/13222

This switches the following files to `IOContext.RANDOM`:
- Stored fields data file.
- Term vectors data file.
- HNSW graph.
- Temporary file storing vectors at merge time that we use to construct the merged HNSW graph.
- Vector data files, including quantized data files.

I hesitated using `IOContext.RANDOM` on terms, since they have a random access pattern when running term queries, but a more sequential access pattern when running multi-term queries. I erred on the conservative side and did not switch them to `IOContext.RANDOM` for now.

For simplicity, I'm only touching the current codec, not previous codecs. There are also some known issues:
- These files will keep using a `RANDOM` `IOContext` at merge time. We need some way for merge instances to get an updated `IOContext`? We have the same problem with `IOContext#LOAD` today.
- With quantized vectors, raw vectors don't have random access pattern, but it was challenging to give raw vectors a sequential access pattern when there are quantized vectors and a random access pattern otherwise. So they assume a random access pattern all the time.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `ReadAdvice#RANDOM` when appropriate. [lucene]

Posted by "jpountz (via GitHub)" <gi...@apache.org>.

jpountz commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2037674709

   I'll look into porting this work to branch_9x via a separate PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `ReadAdvice#RANDOM` when appropriate. [lucene]

Posted by "jpountz (via GitHub)" <gi...@apache.org>.

jpountz commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2037703765

   This is exactly what I'm doing. :) All in the same PR, but it shouldn't be too big.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `IOContext#RANDOM` when appropriate. [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.

uschindler commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2022357798

   In addition, we should still look into getting the IOContexts correct when we merge. The current solution is not ideal, but somehow not really changeable. When you clone an indexinput, you can't set another read advice, as the underlying mmaped segments are the same in the clone.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `ReadAdvice#RANDOM` when appropriate. [lucene]

Posted by "jpountz (via GitHub)" <gi...@apache.org>.

jpountz commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2037833835

   Backport PR: #13267.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `IOContext#RANDOM` when appropriate. [lucene]

Posted by "jpountz (via GitHub)" <gi...@apache.org>.

jpountz commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2023049088

   I opened #13229, is it what you had in mind?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `ReadAdvice#RANDOM` when appropriate. [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.

uschindler commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2037699875

   > I'll look into porting this work to branch_9x via a separate PR.
   
   We should maybe also port some recent changes back, too. Not sure how to best do this. The new record classes and enums make this hard.
   
   Maybe easiest is to use a method like `IOContext#withRandomAccess()`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `IOContext#RANDOM` when appropriate. [lucene]

Posted by "jpountz (via GitHub)" <gi...@apache.org>.

jpountz commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2022902471

   > What do you think?
   
   I had similar thoughts is mind, so that sounds good to me.
   
   I'm still curious about how to fix the bigger issue wrt reader pooling. Should `getMergeInstance()` open fresh new `IndexInput`s? Or maybe we avoid reusing readers across searching and merging, and we'll open a fresh new reader if we need to merge, even though the same segment may already be open for search?
   
   > After merging the file is closed and deleted later. Then the pages get freed automatically, so I don't understand the whole issue that Mike has seen.
   
   This only happens after the whole merge is finished. So if you're merging a 5GB segment, and it takes time, it's still a fair amount of page cache that merges are taking from searches. If pages could be more aggressively reclaimed while the merge is running, this could help?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `IOContext#RANDOM` when appropriate. [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.

uschindler commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2022362746

   About the announced comment: When we merge we want to use sequential, as the kernel may earlier free the pages. But actually I am not sure, if we really need this: After merging the file is closed and deleted later. Then the pages get freed automatically, so I don't understand the whole issue that Mike has seen.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `ReadAdvice#RANDOM` when appropriate. [lucene]

Posted by "jpountz (via GitHub)" <gi...@apache.org>.

jpountz merged PR #13222:
URL: https://github.com/apache/lucene/pull/13222


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `IOContext#RANDOM` when appropriate. [lucene]

Posted by "uschindler (via GitHub)" <gi...@apache.org>.

uschindler commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2022255027

   Hi, I have some problem regarding merging with it - and a suggestion. Please hold with merging.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `IOContext#RANDOM` when appropriate. [lucene]

Posted by "jpountz (via GitHub)" <gi...@apache.org>.

jpountz commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2022301350

   No worries, Uwe. Looking forward to your suggestions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org

Re: [PR] Use `ReadAdvice#RANDOM` when appropriate. [lucene]

Posted by "jpountz (via GitHub)" <gi...@apache.org>.

jpountz commented on PR #13222:
URL: https://github.com/apache/lucene/pull/13222#issuecomment-2037526473

   @uschindler I'm curious what you think of this PR now that `ReadAdvice#RANDOM` is the default. Should we keep calling `state.context.withReadAdvice(ReadAdvice.RANDOM)` to be explicit about the expected access pattern (what the PR is doing now) or should we lean on the default being `ReadAdvice.RANDOM`? In the latter case, we would only need to keep the changes in `Lucene99FlatVectorsWriter`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org