ben-manes commented on a change in pull request #230:
URL: https://github.com/apache/solr/pull/230#discussion_r685465632
##########
File path: solr/core/src/java/org/apache/solr/search/CaffeineCache.java
##########
@@ -176,33 +198,95 @@ public V get(K key) {
return cache.getIfPresent(key);
}
- @Override
- public V computeIfAbsent(K key, Function<? super K, ? extends V>
mappingFunction) {
- return cache.get(key, k -> {
- inserts.increment();
- V value = mappingFunction.apply(k);
- if (value == null) {
- return null;
+ private V computeAsync(K key, IOFunction<? super K, ? extends V>
mappingFunction) throws IOException {
+ CompletableFuture<V> future = new CompletableFuture<>();
+ CompletableFuture<V> result = asyncCache.asMap().putIfAbsent(key, future);
+ lookups.increment();
+ if (result != null) {
+ try {
+ // Another thread is already working on this computation, wait for
them to finish
+ V value = result.join();
+ hits.increment();
+ return value;
+ } catch (CompletionException e) {
+ Throwable cause = e.getCause();
+ if (cause instanceof IOException) {
+ // Computation had an IOException, likely index problems, so fail
this result too
+ throw (IOException) cause;
+ }
+ if (cause instanceof CancellableCollector.QueryCancelledException) {
+ // The reserved slot that we were waiting for got cancelled, so we
will compute directly
+ // If we go back to waiting for a new cache result then that can
lead to thread starvation
+ // Should we record a cache miss here?
+ return mappingFunction.apply(key);
+ }
+ throw e;
}
- ramBytes.add(RamUsageEstimator.sizeOfObject(key,
RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED) +
- RamUsageEstimator.sizeOfObject(value,
RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED));
- ramBytes.add(RamUsageEstimator.LINKED_HASHTABLE_RAM_BYTES_PER_ENTRY);
+ }
+ try {
+ // We reserved the slot, so we do the work
+ V value = mappingFunction.apply(key);
+ future.complete(value); // This will update the weight and expiration
+ recordRamBytes(key, null, value);
+ inserts.increment();
return value;
- });
+ } catch (RuntimeException | IOException e) {
+ // TimeExceeded exception is runtime and will bubble up from here
+ future.completeExceptionally(e); // This will remove the future from the
cache
+ throw e;
+ }
+ }
+
+ @Override
+ public V computeIfAbsent(K key, IOFunction<? super K, ? extends V>
mappingFunction) throws IOException {
+ if (async) {
+ 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();
+ }
}
@Override
public V put(K key, V val) {
inserts.increment();
V old = cache.asMap().put(key, val);
+ cache.put(key, val);
Review comment:
remove redundant put?
--
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
