SOLR-17340: add cache on top of BeanInfo instrospector #2538
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
psalagnac
changed the title
psalagnac
changed the title
dsmiley
changed the title
dsmiley
reviewed
Jul 20, 2024
Comment on lines
-830
to
+848
| MetricUtils.addMXBeanMetrics(obj, intf, null, consumer); | ||
| MetricUtils.addMXBeanMetrics(obj, intf, prefix, consumer); |
There was a problem hiding this comment.
What's this about here; seems like metrics would be published in a different spot, right?
There was a problem hiding this comment.
I forgot to comment on this when creating the pull request.
I think this was a typo when this code was initially added. The prefix parameter of the method is not passed through to the other method. In practice, all callers to addMXBeanMetrics() in Solr code set null for this parameter, so this will not change anything.
Another option could be to just remove the parameter, but I think it's better to keep it if we have later more categories for metrics.
psalagnac
changed the title
dsmiley
reviewed
Jul 22, 2024
| @@ -152,6 +152,8 @@ Optimizations | |||
|
|
|||
| * SOLR-17349: SolrDocumentFetcher should always skip lazy field loading overhead if documentCache==null (Michael Gibney) | |||
|
|
|||
| * JIRA-17340: Add cache on top of system metrics BeanInfos to make call to /admin/info/system faster (Pierre Salagnac) | |||
There was a problem hiding this comment.
Suggested change
| * JIRA-17340: Add cache on top of system metrics BeanInfos to make call to /admin/info/system faster (Pierre Salagnac) | |
| * SOLR-17340: Add cache on top of system metrics BeanInfos to make call to /admin/info/system faster (Pierre Salagnac) |
dsmiley
pushed a commit
that referenced
this pull request
Jul 22, 2024
Note the prefix arg was always null, thus this one-line change is just a refactoring for correctness. (cherry picked from commit 0da7a78)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/SOLR-17340
Description
Solr endpoint
/solr/admin/info/systemis slow because of time spent in Java beans introspection.Solution
Add a simple cache on top of bean info introspection. For each bean class, the introspection will be run only once for the JVM lifespan.
Tests
Naive performance benchmark. See results in Jira comment
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.