SOLR-17160: Time based tracking of core admin requests with Caffeine cache by psalagnac · Pull Request #2304 · apache/solr

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

  }
+
+  @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 dev@solr.apache.org mailing list. Thank you for your contribution!

…ler.java Co-authored-by: David Smiley <dsmiley@apache.org>

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

     */
-    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 <dsmiley@salesforce.com> (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.

…ache#2304) 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 <dsmiley@salesforce.com>

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 <dsmiley@apache.org>

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

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

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

Conversation

psalagnac commented Feb 26, 2024

Description

Solution

Tests

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dsmiley Mar 2, 2024

Choose a reason for hiding this comment

Uh oh!

psalagnac Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dsmiley Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dsmiley Mar 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented May 8, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dsmiley Jun 28, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dsmiley commented Jul 9, 2024

Uh oh!

dsmiley commented Jul 9, 2024

Uh oh!

dsmiley commented Jul 12, 2024

Uh oh!

psalagnac commented Jul 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

psalagnac Mar 4, 2024 •

edited

Loading