search

apache / lucene

Apache Lucene open-source search software
https://lucene.apache.org/
Apache License 2.0
2.46k stars 979 forks source link

Make the default ReadAdvice configurable by sysprop #13264

Closed uschindler closed 2 months ago

uschindler commented 2 months ago

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.

uschindler commented 2 months ago

P.S.: I moved the lookup cache to a new constant name and placed it next to the withReadAdvice() method.

uschindler commented 2 months ago

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 codecs when opening 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 .