SOLR-17337: Show proper distributed stage id #2526
Conversation
- Shows a proper distributed stage id instead of an empty string
|
Triggered the GitHub jobs. Is this too trivial to need a test? |
|
There's only one valid answer to this, I'll add a test 👍 |
|
I think it really ends up being an integration test showing the behavior works? |
|
I added a simple resolution test. Testing the distributed behaviour would mean adding a new test Solr configuration and adding a new test component that actually does a couple of distributed requests in custom stages :-/ |
|
I assigned it to me so I don't loose track of it! |
| @@ -181,16 +183,28 @@ public void modifyRequest(ResponseBuilder rb, SearchComponent who, ShardRequest | |||
| } | |||
| } | |||
|
|
|||
| @VisibleForTesting | |||
| static String getDistributedStageName(int stage) { | |||
There was a problem hiding this comment.
minor/subjective: making this non-static so that custom debug components could override to use an alternative (more meaningful) stage name
| static String getDistributedStageName(int stage) { | |
| protected String getDistributedStageName(int stage) { |
There was a problem hiding this comment.
#2594 opened with the changes in this PR plus making the new method protected with matching test changes.
The config APIs could be used instead of having another test Solr configuration -- e.g. https://github.com/apache/solr/blob/releases/solr/9.6.1/solr/core/src/test/org/apache/solr/handler/component/CustomHighlightComponentTest.java -- but yeah that would be quite a bit of code compared to a simple resolution test. |
|
I think we need a |
|
|
||
| @Test | ||
| public void testDistributedStageResolution() throws IOException { | ||
| assertEquals("PARSE_QUERY", DebugComponent.getDistributedStageName(ResponseBuilder.STAGE_PARSE_QUERY)); |
There was a problem hiding this comment.
gradlew tidy
| assertEquals("PARSE_QUERY", DebugComponent.getDistributedStageName(ResponseBuilder.STAGE_PARSE_QUERY)); | |
| assertEquals("PARSE_QUERY", | |
| DebugComponent.getDistributedStageName(ResponseBuilder.STAGE_PARSE_QUERY)); |
edit: I just tried to in-browser add the tidy fix here but it won't let me, likely because the branch is from an org rather than user fork of the repo, known git issue somewhere.
@tboeghk - would you have a moment to press "commit suggestion" here? thanks!
|
closing in favour of #2594 variant |
https://issues.apache.org/jira/browse/SOLR-17337
Description
When showing track debug for custom stages in distributed requests, the stage id is not translated into a meaningful string. It is displayed as an empty string (see attachment). Furthermore, if you have multiple custom stages, the details for the most recent stage is displayed. The details for other stages is omitted.
Solution
If a given stage has no translation in the
stagemap, we format it's name in the formatSTAGE_<STAGE_ID>. By doing so, no stage details are omitted, even if you have multiple custom stages.Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.