Skip to content

Feat/query job refactor - #5812

Closed
penghuo wants to merge 14 commits into
opensearch-project:mainfrom
penghuo:feat/query-job-refactor
Closed

Feat/query job refactor#5812
penghuo wants to merge 14 commits into
opensearch-project:mainfrom
penghuo:feat/query-job-refactor

Conversation

@penghuo

@penghuo penghuo commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Description

[Describe what this change achieves]

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • New PPL command checklist all confirmed.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff or -s.
  • Public documentation issue/PR created.

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.

penghuo and others added 14 commits September 24, 2026 22:43
@penghuo
Add PPL asynchronous query lifecycle core
af73e70 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Keep async request parsing in API layer
419de01 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Clarify async query user tests
c2c5674 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Name the unsecured async query identity
7b73c9f 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Split async job state machine tests
eed8cb2 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Refine async query lifecycle state model
712d6e5 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Clarify async job state assertions
557108f 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Verify direct failure removes async job
9050a7f 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Clarify async job lease boundary test
83eaff4 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Use fluent scenarios for async query service tests
b2beea0 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Verify expired async jobs return not found
178b674 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
Clarify deletion of completed async jobs
7abe947 
Signed-off-by: Peng Huo <penghuo@gmail.com>
@penghuo
refactor(async-query): consolidate lifecycle into QueryJob active object
197f446 
Replaces PPLAsyncQueryService (~840 lines) and PPLAsyncQueryJob with a
single active-object design:

- QueryJob owns the state machine, retention deadline, keep-alive expiry
  timer, task registration and cancellation, execution attachment and
  completion cleanup. Public API: create(...), get(keepAlive),
  cancel(reason), getJobId, getOwner. Snapshot / Status / Failure are
  the only external types.
- QueryJobRegistry is a thin ConcurrentMap wrapper with add / get /
  remove / close. close() calls discard() on every job and rejects
  further adds.
- PPLAsyncQueryJobId -> QueryJobId (now public).
- PPLAsyncQueryUser  -> QueryJobOwner (authorize() now public).
- Renamed tests keep prior coverage; PPLAsyncQueryServiceTest and
  PPLAsyncQueryJobTest are removed together with the service.

AdmissionControl (running/retained capacity limits) and request
validation are deferred to a follow-up PR, matching the current PR
description's boundary.

Signed-off-by: Peng Huo <penghuo@amazon.com>
@penghuo
test(async-query): fluent-scenario tests for QueryJob and QueryJobReg…
f3a125e 
…istry

QueryJobTest drives one QueryJob through its lifecycle using a fake
clock and a fake Scheduler. Covers direct success/failure, retention
timeout, retained success/failure with metric recording, DELETE
statuses, expiry via GET boundary and via expiry timer, keep-alive
renewal, shutdown discard, authorization, and late-attach after DELETE.

QueryJobRegistryTest covers add / remove / close semantics and the
defensive jobs() snapshot.

Signed-off-by: Peng Huo <penghuo@amazon.com>
@github-actions

github-actions Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

PR Code Analyzer ❗

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

PathLineSeverityDescription
plugin/build.gradle171highNew dependency added: 'org.opensearch:common-utils:${opensearch_build}'. Per mandatory policy, all dependency additions must be flagged regardless of apparent legitimacy. Maintainers should verify the artifact origin, integrity, and that the version variable resolves to the expected trusted artifact.

The table above displays the top 10 most important findings.

Total: 1 | Critical: 0 | High: 1 | 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.

@codecov

codecov Bot commented Sep 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.26%. Comparing base (f2f4893) to head (f3a125e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...c/main/java/org/opensearch/sql/ppl/PPLService.java 50.00% 4 Missing ⚠️
...opensearch/sql/ppl/DefaultAsyncQueryExecution.java 90.00% 1 Missing ⚠️

❌ Your project check has failed because the head coverage (63.26%) is below the target coverage (99.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #5812      +/-   ##
============================================
+ Coverage     63.23%   63.26%   +0.02%     
- Complexity     8823     8829       +6     
============================================
  Files           938      939       +1     
  Lines         40236    40276      +40     
  Branches       4537     4537              
============================================
+ Hits          25445    25480      +35     
- Misses        13966    13971       +5     
  Partials        825      825
Flag Coverage Δ
sql-engine 63.26% <87.50%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@penghuo penghuo closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ps48 ps48 Awaiting requested review from ps48 ps48 is a code owner
@joshuali925 joshuali925 Awaiting requested review from joshuali925 joshuali925 is a code owner
@dai-chen dai-chen Awaiting requested review from dai-chen dai-chen is a code owner
@mengweieric mengweieric Awaiting requested review from mengweieric mengweieric is a code owner
@vamsimanohar vamsimanohar Awaiting requested review from vamsimanohar vamsimanohar is a code owner
@Swiddis Swiddis Awaiting requested review from Swiddis Swiddis is a code owner
@anirudha anirudha Awaiting requested review from anirudha anirudha is a code owner
@acarbonetto acarbonetto Awaiting requested review from acarbonetto acarbonetto is a code owner
@ykmr1224 ykmr1224 Awaiting requested review from ykmr1224 ykmr1224 is a code owner
@LantaoJin LantaoJin Awaiting requested review from LantaoJin LantaoJin is a code owner
@noCharger noCharger Awaiting requested review from noCharger noCharger is a code owner
@qianheng-aws qianheng-aws Awaiting requested review from qianheng-aws qianheng-aws is a code owner
@yuancu yuancu Awaiting requested review from yuancu yuancu is a code owner
@RyanL1997 RyanL1997 Awaiting requested review from RyanL1997 RyanL1997 is a code owner
@ahkcs ahkcs Awaiting requested review from ahkcs ahkcs is a code owner
@songkant-aws songkant-aws Awaiting requested review from songkant-aws songkant-aws is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@penghuo