magibney commented on a change in pull request #592:
URL: https://github.com/apache/solr/pull/592#discussion_r808365564
##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -854,20 +880,92 @@ private DocSet getAndCacheDocSet(Query query) throws
IOException {
return filterCache.computeIfAbsent(query, q -> getDocSetNC(q, null));
}
- private static Query matchAllDocsQuery = new MatchAllDocsQuery();
- private volatile BitDocSet liveDocs;
+ private static final MatchAllDocsQuery MATCH_ALL_DOCS_QUERY = new
MatchAllDocsQuery();
+
+ /**
+ * A naively cached canonical `liveDocs` DocSet. This does not need to be
volatile. It may be set multiple times,
+ * but should always be set to the same value, as all set values should pass
through `liveDocsCache.computeIfAbsent`
+ */
+ private BitDocSet liveDocs;
+ private final IOFunction<MatchAllDocsQuery, BitDocSet> computeLiveDocs =
this::computeLiveDocs;
+
+ private static final BitDocSet EMPTY = new BitDocSet(new FixedBitSet(0), 0);
+
+ private BitDocSet computeLiveDocs(Query q) {
+ assert q == MATCH_ALL_DOCS_QUERY;
Review comment:
I guess the thought was that this method looks tempting to call ...
"computeLiveDocs? Sure I wanna do that!". It's really supposed to be very
locked-down in terms of the contexts from which it's called (and even then not
called directly -- it's called via CaffeinCache.computeIfAbsent).
Reverse-engineering my subconscious thought process here: normally `private`
access modifier alone would take care of that, but in a class as sprawling as
SIS the power/semantics of `private` risk being diluted. Could change the
method signature to MatchAllDocsQuery, yes; but paradoxically that might make
it seem _more_ like it can be called by anyone with a MatchAllDocsQuery.
If this logic makes sense I'll add a comment explaining this in a more
succinct way. I don't mind removing the assertion, if you prefer.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
