Skip to content

Commit

Permalink
Partial fix for #12681 caching content (#12702)
Browse files Browse the repository at this point in the history
This is a partial fix for #12681. It does not fix the actual release race on a single content, but by restricting the shrinking to a single thread, make the race less likely to occur.

There is a more comprehensive fix in #12678 and #12704 for 12.1,
  • Loading branch information
gregw authored Jan 13, 2025
1 parent 0a23b82 commit 73e6058
Showing 1 changed file with 37 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public class CachingHttpContentFactory implements HttpContent.Factory
private final HttpContent.Factory _authority;
private final ConcurrentHashMap<String, CachingHttpContent> _cache = new ConcurrentHashMap<>();
private final AtomicLong _cachedSize = new AtomicLong();
private final AtomicBoolean _shrinking = new AtomicBoolean();
private final ByteBufferPool _bufferPool;
private int _maxCachedFileSize = DEFAULT_MAX_CACHED_FILE_SIZE;
private int _maxCachedFiles = DEFAULT_MAX_CACHED_FILES;
Expand Down Expand Up @@ -148,35 +149,46 @@ public void setUseDirectByteBuffers(boolean useDirectByteBuffers)

private void shrinkCache()
{
// While we need to shrink
int numCacheEntries = _cache.size();
while (numCacheEntries > 0 && (numCacheEntries > _maxCachedFiles || _cachedSize.get() > _maxCacheSize))
// Only 1 thread shrinking at once
if (_shrinking.compareAndSet(false, true))
{
// Scan the entire cache and generate an ordered list by last accessed time.
SortedSet<CachingHttpContent> sorted = new TreeSet<>((c1, c2) ->
try
{
long delta = NanoTime.elapsed(c2.getLastAccessedNanos(), c1.getLastAccessedNanos());
if (delta != 0)
return delta < 0 ? -1 : 1;

delta = c1.getContentLengthValue() - c2.getContentLengthValue();
if (delta != 0)
return delta < 0 ? -1 : 1;

return c1.getKey().compareTo(c2.getKey());
});
sorted.addAll(_cache.values());

// TODO: Can we remove the buffers from the content before evicting.
// Invalidate least recently used first
for (CachingHttpContent content : sorted)
// While we need to shrink
int numCacheEntries = _cache.size();
while (numCacheEntries > 0 && (numCacheEntries > _maxCachedFiles || _cachedSize.get() > _maxCacheSize))
{
// Scan the entire cache and generate an ordered list by last accessed time.
SortedSet<CachingHttpContent> sorted = new TreeSet<>((c1, c2) ->
{
long delta = NanoTime.elapsed(c2.getLastAccessedNanos(), c1.getLastAccessedNanos());
if (delta != 0)
return delta < 0 ? -1 : 1;

delta = c1.getContentLengthValue() - c2.getContentLengthValue();
if (delta != 0)
return delta < 0 ? -1 : 1;

return c1.getKey().compareTo(c2.getKey());
});
sorted.addAll(_cache.values());

// TODO: Can we remove the buffers from the content before evicting.
// Invalidate least recently used first
for (CachingHttpContent content : sorted)
{
if (_cache.size() <= _maxCachedFiles && _cachedSize.get() <= _maxCacheSize)
break;
removeFromCache(content);
}

numCacheEntries = _cache.size();
}
}
finally
{
if (_cache.size() <= _maxCachedFiles && _cachedSize.get() <= _maxCacheSize)
break;
removeFromCache(content);
_shrinking.set(false);
}

numCacheEntries = _cache.size();
}
}

Expand Down

0 comments on commit 73e6058

Please sign in to comment.