dsmiley commented on a change in pull request #230:
URL: https://github.com/apache/solr/pull/230#discussion_r676293197
##########
File path: solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
##########
@@ -804,15 +807,29 @@ public BitDocSet getDocSetBits(Query q) throws
IOException {
}
// only handle positive (non negative) queries
- DocSet getPositiveDocSet(Query q) throws IOException {
- DocSet answer;
- if (filterCache != null) {
- answer = filterCache.get(q);
- if (answer != null) return answer;
+ DocSet getPositiveDocSet(Query query) throws IOException {
+ // TODO duplicated code with getDocSet, can maybe also use FutureDocSet
here
+ boolean doCache = filterCache != null;
Review comment:
I like this `doCache` local var for clarity but otherwise I don't see
why you are making any changes to this method. It also appears getDocSetNC
might get called twice if there's a cache miss?
##########
File path:
solr/core/src/test/org/apache/solr/request/TestUnInvertedFieldException.java
##########
@@ -96,13 +96,9 @@ public void testConcurrentInit() throws Exception {
List<Future<UnInvertedField>> futures = initCallables.stream().map((c)
-> pool.submit(c))
.collect(Collectors.toList());
for (Future<UnInvertedField> uifuture : futures) {
- try {
- final UnInvertedField uif = uifuture.get();
- } catch (ExecutionException injection) {
- SolrException solrException = (SolrException) injection.getCause();
- assertEquals(ErrorCode.SERVER_ERROR.code, solrException.code());
- assertSame(solrException.getCause().getClass(),
OutOfMemoryError.class);
- }
+ ExecutionException injection =
assertThrows(ExecutionException.class, uifuture::get);
Review comment:
Nice
##########
File path: solr/core/src/java/org/apache/solr/search/SolrCache.java
##########
@@ -106,8 +108,23 @@
* @return existing or newly computed value, null if there was no existing
value and
* it was not possible to compute a new value (in which case the new mapping
won't be created).
*/
+ // Can we deprecate/remove this in favor of the other signature?
public V computeIfAbsent(K key, Function<? super K, ? extends V>
mappingFunction);
+ default V computeIfAbsent(K key, IOFunction<? super K, ? extends V>
mappingFunction) throws IOException {
Review comment:
I like this; it simplified some of the callers nicely.
##########
File path: solr/core/src/java/org/apache/solr/search/SolrCache.java
##########
@@ -106,8 +108,23 @@
* @return existing or newly computed value, null if there was no existing
value and
* it was not possible to compute a new value (in which case the new mapping
won't be created).
*/
+ // Can we deprecate/remove this in favor of the other signature?
Review comment:
Yes
##########
File path: solr/core/src/java/org/apache/solr/query/SolrRangeQuery.java
##########
@@ -452,6 +455,7 @@ private SegState getSegState(LeafReaderContext context)
throws IOException {
if (doCheck) {
answer = createDocSet(solrSearcher, count);
+ // This can be a naked put because the cache usually gets checked in
SolrIndexSearcher
Review comment:
I don't follow your meaning, nor what a "naked put" is
##########
File path:
solr/core/src/java/org/apache/solr/schema/RptWithGeometrySpatialField.java
##########
@@ -163,10 +164,11 @@ public Shape value() throws IOException {
throw new IllegalStateException("Leaf " + readerContext.reader() +
" is not suited for caching");
}
PerSegCacheKey key = new PerSegCacheKey(cacheHelper.getKey(), docId);
- Shape shape = cache.computeIfAbsent(key, k -> {
+ Shape shape = cache.computeIfAbsent(key, (Function<PerSegCacheKey,
Shape>) k -> {
try {
return targetFuncValues.value();
} catch (IOException e) {
+ // Strange that this returns null instead of propagating
exceptions
Review comment:
Lets propagate it. I am guessing maybe I was lazy here :-(
--
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-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
