Skip to content

Stub support for streaming-transport request handlers in MockTransportService#21581

Merged
rishabhmaurya merged 3 commits into
opensearch-project:mainfrom
mch2:coordinator-stubbable
May 11, 2026
Merged

Stub support for streaming-transport request handlers in MockTransportService#21581
rishabhmaurya merged 3 commits into
opensearch-project:mainfrom
mch2:coordinator-stubbable

Conversation

@mch2
Copy link
Copy Markdown
Member

@mch2 mch2 commented May 9, 2026

Description

This change adds stub support for InternalClusterTests to stub streaming responses in tests. Also includes a sandbox/qa module that uses it with a simple test case, a bug it uncovered, and a deterministic unit test for the fix.

Related Issues

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…tService

Adds Node.wrapStreamTransport hook so MockNode can wrap the streaming
transport before it's shared between TransportService and
StreamTransportService. MockTransportService.addRequestHandlingBehavior
falls back to the streaming-transport's stub registry when the action
isn't on the regular transport — needed for streaming-only handlers
(e.g. when FlightStreamPlugin is loaded).

Demonstrated by a new sandbox/qa/analytics-engine-coordinator IT that
stubs FragmentExecutionAction (streaming-only) end-to-end, plus a
deterministic unit test for a lookahead-reordering bug surfaced via
this infrastructure: the listener was offloading onStreamResponse
bodies to a thread pool, letting the isLast=true task race ahead of
earlier batches and drop them via the SUCCEEDED short-circuit.

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
@mch2 mch2 requested a review from a team as a code owner May 9, 2026 20:49
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit a11e140.

PathLineSeverityDescription
sandbox/qa/analytics-engine-coordinator/build.gradle58highNew external runtime dependencies added: com.google.guava:guava:33.3.1-jre and com.google.guava:failureaccess:1.0.1. Per supply chain policy, all new dependency additions must be verified for artifact authenticity regardless of package name legitimacy.
sandbox/qa/analytics-engine-coordinator/build.gradle64highNew detached Gradle configuration injects multiple external artifacts (guava, failureaccess, slf4j, commons-codec, jackson-core, jackson-databind, jackson-annotations) directly onto the runtime classpath outside normal dependency resolution, bypassing inherited exclusions. All artifact hashes must be verified.
sandbox/qa/analytics-engine-coordinator/build.gradle75highresolutionStrategy.force pins versions for over 30 external dependencies including security-sensitive libraries (log4j, jackson, protobuf, okhttp, kotlin-stdlib, snakeyaml, commons-codec). Any pin to a vulnerable or compromised version would silently override transitive resolution across all configurations.
sandbox/qa/analytics-engine-coordinator/build.gradle73highTransitive dependency com.github.babbel:okhttp-aws-signer is explicitly excluded globally via configurations.all. Removing a dependency changes the effective dependency graph and must be reviewed to confirm no security-relevant artifact is being suppressed.

The table above displays the top 10 most important findings.

Total: 4 | Critical: 0 | High: 4 | Medium: 0 | Low: 0


Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.


⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

@mch2 mch2 added skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. skip-diff-reviewer Maintainer to skip code-diff-reviewer check, after reviewing issues in AI analysis. labels May 9, 2026
@mch2
Copy link
Copy Markdown
Member Author

mch2 commented May 9, 2026

new package adds / deps are for new sandbox/qa module to stress test coordinator e2e

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

❌ Gradle check result for a11e140: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Marc Handalian <marc.handalian@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

✅ Gradle check result for b2d66a4: SUCCESS

@codecov
Copy link
Copy Markdown

codecov Bot commented May 10, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.50%. Comparing base (8f72a95) to head (b2d66a4).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
server/src/main/java/org/opensearch/node/Node.java 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #21581      +/-   ##
============================================
+ Coverage     73.48%   73.50%   +0.01%     
+ Complexity    74646    74602      -44     
============================================
  Files          5980     5980              
  Lines        338777   338778       +1     
  Branches      48848    48848              
============================================
+ Hits         248964   249016      +52     
+ Misses        70026    69922     -104     
- Partials      19787    19840      +53     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bowenlan-amzn bowenlan-amzn removed the skip-diff-reviewer Maintainer to skip code-diff-reviewer check, after reviewing issues in AI analysis. label May 11, 2026
@rishabhmaurya rishabhmaurya merged commit d790b71 into opensearch-project:main May 11, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants