dsmiley commented on code in PR #1481:
URL: https://github.com/apache/solr/pull/1481#discussion_r1145419650
##########
solr/core/src/java/org/apache/solr/search/CaffeineCache.java:
##########
@@ -250,26 +249,35 @@ public V computeIfAbsent(K key, IOFunction<? super K, ?
extends V> mappingFuncti
return computeAsync(key, mappingFunction);
}
- try {
- return cache.get(
- key,
- k -> {
- V value;
- try {
- value = mappingFunction.apply(k);
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- if (value == null) {
- return null;
- }
- recordRamBytes(key, null, value);
- inserts.increment();
- return value;
- });
- } catch (UncheckedIOException e) {
- throw e.getCause();
+ /*
+ Synchronous caches must effectively use get-then-put under the hood in
place of
+ `computeIfAbsent()`-type behavior. A number of Solr queries (and
consequently,
+ `mappingFunction`s passed into this method) would modify the cache
recursively.
+ This is supported in `async` mode, but when `async==false`, it may yield
+ "IllegalStateException: Recursive update" (see SOLR-16707).
+ */
+ V cached = cache.getIfPresent(key);
+ if (cached != null) {
+ return cached;
+ }
+ final V computed = mappingFunction.apply(key);
Review Comment:
Computing the value outside the cache means a potentially expensive query
that is requested concurrently will be computed redundantly. I think that's
the value that was added previously, now defeated. Maybe we can get this
behavior back by always doing async mode in 10?
CC @madrob who I think added this
--
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
