Skip to content

Avoid some uses of ConcurrentLinkedQueue.size() in SharedBlobCacheService (#128119) #128194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 20, 2025
Merged

Avoid some uses of ConcurrentLinkedQueue.size() in SharedBlobCacheService (#128119) #128194

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions ...n/blob-cache/src/main/java/org/elasticsearch/blobcache/shared/SharedBlobCacheService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ public boolean maybeFetchFullEntry(
ActionListener<Void> listener
) {
int finalRegion = getEndingRegion(length);
// TODO freeRegionCount uses freeRegions.size() which is is NOT a constant-time operation. Can we do better?
if (freeRegionCount() < finalRegion) {
// Not enough room to download a full file without evicting existing data, so abort
listener.onResponse(null);
Expand Down Expand Up @@ -571,7 +572,7 @@ public void maybeFetchRegion(
final Executor fetchExecutor,
final ActionListener<Boolean> listener
) {
if (freeRegionCount() < 1 && maybeEvictLeastUsed() == false) {
if (freeRegions.isEmpty() && maybeEvictLeastUsed() == false) {
// no free page available and no old enough unused region to be evicted
logger.info("No free regions, skipping loading region [{}]", region);
listener.onResponse(false);
Expand Down Expand Up @@ -619,7 +620,7 @@ public void maybeFetchRange(
final Executor fetchExecutor,
final ActionListener<Boolean> listener
) {
if (freeRegionCount() < 1 && maybeEvictLeastUsed() == false) {
if (freeRegions.isEmpty() && maybeEvictLeastUsed() == false) {
// no free page available and no old enough unused region to be evicted
logger.info("No free regions, skipping loading region [{}]", region);
listener.onResponse(false);
Expand Down Expand Up @@ -671,7 +672,11 @@ private static void throwAlreadyClosed(String message) {
throw new AlreadyClosedException(message);
}

// used by tests
/**
* NOTE: Method is package private mostly to allow checking the number of fee regions in tests.
* However, it is also used by {@link SharedBlobCacheService#maybeFetchFullEntry} but we should try
* to move away from that because calling "size" on a ConcurrentLinkedQueue is not a constant time operation.
*/
int freeRegionCount() {
return freeRegions.size();
}
Expand Down