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

psalagnac · 2024-02-26T22:30:39Z

https://issues.apache.org/jira/browse/SOLR-17160

This is a simpler version of #2271, that now uses Caffeine cache instead of manually dropping old requests.

Description

Core admin completed/failed requests are tracked in-memory, but we only track a maximum of 100 requests by dropping the oldest ones. If for any reason we drop a request before the client (an other Solr node) fetched the status, we will get unexpected errors at the top level command.

Solution

Instead of the hard limit of 100 tracked requests, we track all requests in a Caffeine cache, configured to drop old requests.
This is a trade-off to not keep memory forever and to be sure we won't lose status in the case of many incoming requests.

The change adds two different timeouts:

an hour for every requests (overridable with property "solr.admin.requests.running.timeout.minutes")
5 minutes for completed/failed requests once the status is already retrieved by client (overridable with property "solr.admin.requests.completed.timeout.minutes")

Tests

Added a unit test in CoreAdminHandlerTest to check expired requests are not tracked in-memory anymore.

Also checks we don't have any issue for collection backup with very high numbers of shards.

Checklist

Please review the following and check all that apply:

I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
I have created a Jira issue and added the issue ID to my pull request title.
I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
I have developed this patch against the main branch.
I have run ./gradlew check.
I have added tests for my changes.
I have added documentation for the Reference Guide

…cache

solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java

dsmiley · 2024-03-02T03:33:31Z

solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeCommandStatus.java

+        requestStatusResponse.response = taskObject.getRspObject();
+        requestStatusResponse.response = taskObject.getOperationRspObject();


I'm confused; you are setting the response twice?

Oh, good catch!
Something is definitely wrong here, but this is existing code. I did not notice that.
I think this deserves a a fix by itself. Values for msg and response in the V2 API response are not consistent depending on the status. And it seems to me they're not consistent with V1 either.
I think this was introduced with #2144.

Maybe I should put this PR on hold and fix this first?

Ah; didn't know it was existing code. Do in whichever order you want; it doesn't really matter.

solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeCommandStatus.java

dsmiley · 2024-03-02T03:36:50Z

solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java

@@ -528,4 +534,39 @@ public void testNonexistentCoreReload() throws Exception {
        e.getMessage());
    admin.close();
  }
+
+  @Test
+  public void testTrackedRequestExpiration() throws Exception {


wonderful test

solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java

github-actions · 2024-05-08T00:00:27Z

This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the [email protected] mailing list. Thank you for your contribution!

…ler.java Co-authored-by: David Smiley <[email protected]>

solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java

dsmiley · 2024-06-28T14:10:18Z

solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java

@@ -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);


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)?

solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java

dsmiley · 2024-07-09T04:46:02Z

WDYT of this proposed CHANGES.txt under Improvements:

SOLR-17160: Core Admin "async" request status tracking is no longer capped at 100; it's 10k. Statuses are now removed 5 minutes after the read of a completed/failed status. (Pierre Salagnac)

dsmiley · 2024-07-09T14:46:05Z

Perhaps an additional sentence to highlight that this helps collection backup/restore and other operations scale to 100+ shards.

dsmiley · 2024-07-12T21:31:13Z

I handled CHANGES.txt myself. I'll merge this sometime this weekend. Looking forward to seeing this one done!

# Conflicts: # solr/CHANGES.txt

) Core Admin "async" request status tracking is no longer capped at 100; it's 10k. Statuses are now removed 5 minutes after the read of a completed/failed status. Helps collection async backup/restore and other operations scale to 100+ shards. Co-authored-by: David Smiley <[email protected]> (cherry picked from commit d3b4c2e)

psalagnac · 2024-07-22T08:42:38Z

I handled CHANGES.txt myself. I'll merge this sometime this weekend. Looking forward to seeing this one done!

Thanks for handling this @dsmiley. I was off for some time and unable to do any change.

SOLR-17160: Time based tracking of core admin requests with Caffeine …

fdfc9f0

…cache

github-actions bot added the tests label Feb 26, 2024

psalagnac mentioned this pull request Feb 26, 2024

SOLR-17160: Time based tracking of core admin requests #2271

Closed

7 tasks

Added max size to the cache

8df6c8c

dsmiley reviewed Mar 2, 2024

View reviewed changes

github-actions bot added the stale PR not updated in 60 days label May 8, 2024

psalagnac and others added 5 commits June 10, 2024 16:42

Update solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHand…

ddd45a6

…ler.java Co-authored-by: David Smiley <[email protected]>

Use EnvUtils

eda2c05

Avoid useless method calls.

b29b498

Rename and document flag.

7e87990

Fix race condition when adding two requests with same name

09c3430

github-actions bot removed the stale PR not updated in 60 days label Jun 12, 2024

dsmiley reviewed Jun 13, 2024

View reviewed changes

psalagnac added 4 commits June 27, 2024 12:21

Merge branch 'main' into max-requests-caffeine

e23d0ba

Replace getEnvAsLong by getPropertyAsLong

525aa35

Add config property for cache size

c439377

Remove duplicate put() to the cache

ec33077

github-actions bot added the client:solrj label Jun 27, 2024

dsmiley reviewed Jun 28, 2024

View reviewed changes

psalagnac added 2 commits July 4, 2024 15:54

Remove useless assignment

9629e2a

Rename config properties

a1b4ad3

dsmiley added 2 commits July 12, 2024 17:04

Merge branch 'refs/heads/main' into fork/max-requests-caffeine

1e350dd

CHANGES.txt

42c2575

Merge branch 'refs/heads/main' into fork/max-requests-caffeine

ee7a896

# Conflicts: # solr/CHANGES.txt

dsmiley merged commit d3b4c2e into apache:main Jul 15, 2024
3 of 4 checks passed

psalagnac deleted the max-requests-caffeine branch July 22, 2024 08:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

psalagnac commented Feb 26, 2024

dsmiley Mar 2, 2024

psalagnac Mar 4, 2024 •

edited

Loading

dsmiley Mar 4, 2024

dsmiley Mar 2, 2024

github-actions bot commented May 8, 2024

dsmiley Jun 28, 2024

dsmiley commented Jul 9, 2024

dsmiley commented Jul 9, 2024

dsmiley commented Jul 12, 2024

psalagnac commented Jul 22, 2024

		requestStatusResponse.response = taskObject.getRspObject();
		requestStatusResponse.response = taskObject.getOperationRspObject();

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

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

Conversation

psalagnac commented Feb 26, 2024

Description

Solution

Tests

Checklist

dsmiley Mar 2, 2024

Choose a reason for hiding this comment

psalagnac Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

dsmiley Mar 4, 2024

Choose a reason for hiding this comment

dsmiley Mar 2, 2024

Choose a reason for hiding this comment

github-actions bot commented May 8, 2024

dsmiley Jun 28, 2024

Choose a reason for hiding this comment

dsmiley commented Jul 9, 2024

dsmiley commented Jul 9, 2024

dsmiley commented Jul 12, 2024

psalagnac commented Jul 22, 2024

psalagnac Mar 4, 2024 •

edited

Loading