Skip to content
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

SOLR-17160: Time based tracking of core admin requests with Caffeine cache #2304

Merged
merged 16 commits into from
Jul 15, 2024
Merged

SOLR-17160: Time based tracking of core admin requests with Caffeine cache #2304

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
fdfc9f0
SOLR-17160: Time based tracking of core admin requests with Caffeine …
psalagnac Feb 26, 2024
8df6c8c
Added max size to the cache
psalagnac Feb 28, 2024
ddd45a6
Update solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHand…
psalagnac Jun 10, 2024
eda2c05
Use EnvUtils
psalagnac Jun 10, 2024
b29b498
Avoid useless method calls.
psalagnac Jun 10, 2024
7e87990
Rename and document flag.
psalagnac Jun 10, 2024
09c3430
Fix race condition when adding two requests with same name
psalagnac Jun 10, 2024
e23d0ba
Merge branch 'main' into max-requests-caffeine
psalagnac Jun 27, 2024
525aa35
Replace getEnvAsLong by getPropertyAsLong
psalagnac Jun 27, 2024
c439377
Add config property for cache size
psalagnac Jun 27, 2024
ec33077
Remove duplicate put() to the cache
psalagnac Jun 27, 2024
9629e2a
Remove useless assignment
psalagnac Jul 4, 2024
a1b4ad3
Rename config properties
psalagnac Jul 4, 2024
1e350dd
Merge branch 'refs/heads/main' into fork/max-requests-caffeine
dsmiley Jul 12, 2024
42c2575
CHANGES.txt
dsmiley Jul 12, 2024
ee7a896
Merge branch 'refs/heads/main' into fork/max-requests-caffeine
dsmiley Jul 15, 2024
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
16 changes: 11 additions & 5 deletions solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,8 @@ public static class CoreAdminAsyncTracker {
* we're not supposed to hit it. This is just a protection to grow in memory too much when
* receiving an abusive number of admin requests.
*/
private static final int MAX_TRACKED_REQUESTS = 10_000;
private static final int MAX_TRACKED_REQUESTS =
EnvUtils.getPropertyAsInteger("solr.admin.requests.running.max", 10_000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is only for "async" requests, maybe the name should incorporate this limit like 'solr.admin.async.max". ("requests" and "running" being kind of implicit maybe)?


public static final String RUNNING = "running";
public static final String COMPLETED = "completed";
Expand All @@ -462,9 +463,9 @@ public CoreAdminAsyncTracker() {
this(
Ticker.systemTicker(),
TimeUnit.MINUTES.toNanos(
EnvUtils.getEnvAsLong("solr.admin.requests.running.timeout.minutes", 60L)),
EnvUtils.getPropertyAsLong("solr.admin.requests.running.timeout.minutes", 60L)),
TimeUnit.MINUTES.toNanos(
EnvUtils.getEnvAsLong("solr.admin.requests.completed.timeout.minutes", 5L)));
EnvUtils.getPropertyAsLong("solr.admin.requests.completed.timeout.minutes", 5L)));
}

/**
Expand Down Expand Up @@ -536,7 +537,13 @@ public void submitAsyncTask(TaskObject taskObject) throws SolrException {

private void addTask(TaskObject taskObject) {
// Ensure task ID is not already in use
TaskObject taskInCache = requestStatusCache.get(taskObject.taskId, n -> taskObject);
TaskObject taskInCache =
requestStatusCache.get(
taskObject.taskId,
n -> {
taskObject.status = RUNNING;
return taskObject;
});

// If we get a different task instance, it means one was already in the cache with the
// same name. Just reject the new one.
Expand All @@ -546,7 +553,6 @@ private void addTask(TaskObject taskObject) {
}

taskObject.status = RUNNING;
psalagnac marked this conversation as resolved.
Show resolved Hide resolved
requestStatusCache.put(taskObject.taskId, taskObject);
}

private void finishTask(TaskObject taskObject, boolean successful) {
Expand Down
3 changes: 3 additions & 0 deletions solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,9 @@ public void testDuplicatedRequestId() {
} catch (SolrException e) {
assertEquals("Duplicate request with the same requestid found.", e.getMessage());
}

assertNotNull(task1.getStatus());
assertNull(task2.getStatus());
} finally {
asyncTracker.shutdown();
}
Expand Down
14 changes: 14 additions & 0 deletions solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,20 @@ private static String camelCaseToDotSeparated(String key) {
}

/** Get property as integer */
public static Integer getPropertyAsInteger(String key) {
return getPropertyAsInteger(key, null);
}

/** Get property as integer, or default value */
public static Integer getPropertyAsInteger(String key, Integer defaultValue) {
String value = getProperty(key);
if (value == null) {
return defaultValue;
}
return Integer.parseInt(value);
}

/** Get property as long */
public static Long getPropertyAsLong(String key) {
return getPropertyAsLong(key, null);
}
Expand Down
Loading