You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "uschindler (via GitHub)" <gi...@apache.org> on 2024/04/04 15:50:07 UTC
[PR] Make the default ReadAdvice configurable by sysprop [lucene]
uschindler opened a new pull request, #13264:
URL: https://github.com/apache/lucene/pull/13264
Followup after #13244: This makes the default `ReadAdvice` for index files configurable via system property `org.apache.lucene.store.defaultReadAdvice`.
I moved the corresponding constant to the `oal.util.Constants` class because this makes reading and error reporting easier as @rmuir and I added some framework to safely support security manager and map those variables (we used this for vectorization).
It would be good to move all reading of system properties to that class so we do not have the splattered AccessControllers everywhere.
--
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] Make the default ReadAdvice configurable by sysprop [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler merged PR #13264:
URL: https://github.com/apache/lucene/pull/13264
--
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] Make the default ReadAdvice configurable by sysprop [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13264:
URL: https://github.com/apache/lucene/pull/13264#issuecomment-2037631425
> This makes sense to me. At first I wondered if we needed this since it's possible to create a `FilterDirectory` that calls `ioContext.withReadAdvice(ReadAdvice.NORMAL)` on `Directory#openIndexInput` or something along these lines. But actually this wouldn't work as well, as this would override explicit decisions made from the code, which is probably not what we want.
That was exactly my problem with making the default hardcoded.
I thought about making it configurable "per directory", but this is too much changes for little benefit. As this is mostly machine specific, a system property that can be passed at command line was my favourite.
In addition, we should still pass `iocontext.withReadAdvice(ReadAdvice.RANDOM)` in our index files, where we know for sure that the access will be fully random (like HNSW), so the other PR you implemented was still fine.
So the default specified by system property is only used for cases where no explicit read advice is given at all .
--
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] Make the default ReadAdvice configurable by sysprop [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #13264:
URL: https://github.com/apache/lucene/pull/13264#issuecomment-2037578866
P.S.: I moved the lookup cache to a new constant name and placed it next to the `withReadAdvice()` method.
--
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