[FEATURE] Cancel and delete async PPL queries

[penghuo]

Description

Add DELETE /_plugins/_async_query/{id} for async PPL jobs. Authorize the caller, forward to the owner node when needed, remove the stored job, and request cooperative cancellation through its PPL execution task. Deleting a job also disarms its retention timer. Async submission rejects a task that cannot provide the expected PPL cancellation hook.

DELETE contract:

• Return HTTP 200 with {"status":"CANCELLED"} when cancellation wins against a running job, or the existing SUCCEEDED / FAILED status for a completed job.
• Subsequent GET and DELETE return 404. Unknown, expired, already-deleted, and absent-owner ids return 404.
• Other users receive 403; custom roles need cluster:admin/opensearch/ql/async_query/delete.
• Concurrent DELETEs have one successful remover. The running-query tests verify bounded search activity after acknowledgment, worker drain, and PIT cleanup.

Additional verification for commit 163015dba2 used Java 21 and seed 5846DE1E20261008:

• The full shared :integ-test:integTestWithSecurity task passed 97 security test methods with zero failures/errors/skips, including both async PPL security ITs. Gradle also reported one successful class/container entry separately from these test methods.
• The new IT checks that a job owner without cluster:admin/opensearch/ql/async_query/delete receives HTTP 403 on both owner and peer nodes, with the completed job still readable. Granting DELETE to the same user then allows deletion through the peer and subsequent GET returns 404.
• spotlessApply, spotlessCheck, and integration-test compilation passed. This follow-up changes only AsyncPPLSecurityIT.java.

Related Issues

Related to #5765.
Follows merged #5832.
Rebased onto upstream including #5847.
Cancellation/fallback follow-up: #5849.
Cross-version transport follow-up: #5801.

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit f45c375)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Possible Issue The eviction timer uses a WeakReference to the job, which may be garbage collected before the TTL expires if no other strong references exist. If the job is collected early, the eviction callback becomes a no-op, leaving the job in the store indefinitely. This defeats the retention policy's purpose. The scenario occurs when a job completes, no external code holds a reference, and GC runs before the TTL timer fires. QueryJobId id = job.id(); // The eviction timer must not retain a deleted job until TTL. WeakReference<QueryJob> retained = new WeakReference<>(job); try { threadPool.schedule( () -> { QueryJob current = retained.get(); if (current != null && store.remove(id, current)) { LOG.debug("Evicted terminal query job [{}] after TTL", id.encode()); } }, Possible Issue The unwrapping logic in the forwarding error handler assumes ExceptionsHelper.unwrapCause returns the innermost cause. If unwrapCause returns a non-Exception Throwable (e.g., an Error), the code wraps it in RuntimeException. However, if the cause chain contains multiple layers of RemoteTransportException or other wrappers, the unwrapping may not reach the real application exception, causing the client to see a transport-layer exception instead of the intended ResourceNotFoundException or QueryJobForbiddenException. This breaks the 404/403 contract when nested wrappers are present. ActionListener<RespT> unwrapping = ActionListener.wrap( listener::onResponse, failure -> { Throwable cause = ExceptionsHelper.unwrapCause(failure); listener.onFailure( cause instanceof Exception ? (Exception) cause : new RuntimeException(cause)); });

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to f45c375

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Handle exceptions in cancellation hook The cancel() method invokes cancelExecution.run() without catching potential exceptions. If the cancellation hook throws an exception, it could prevent proper cleanup or leave the future in an inconsistent state. Wrap the cancellation hook invocation in a try-catch block to ensure robustness. ppl/src/main/java/org/opensearch/sql/ppl/PPLQueryRunner.java [103-108] @Override public void cancel() { - // Unlike cancel(false), this returns true only for the call that performs the transition. if (future.completeExceptionally(new CancellationException("PPL query cancelled"))) { - cancelExecution.run(); + try { + cancelExecution.run(); + } catch (Exception e) { + // Log but don't propagate - the future is already cancelled + LOG.warn("Exception during query cancellation", e); + } } } Suggestion importance[1-10]: 6 __ Why: Valid defensive programming suggestion to prevent exceptions in cancelExecution from disrupting the cancellation flow. The future is already marked cancelled, so catching exceptions here improves robustness without changing semantics.	Low
	Centralize exception translation logic Exception translation is performed in the transport action, but the same exceptions are also thrown by the service layer. This creates a tight coupling where the transport layer must know about all possible service exceptions. Consider handling exception translation at a single layer or using a more generic exception handling mechanism. async-query/src/main/java/org/opensearch/sql/spark/transport/TransportCancelAsyncQueryRequestAction.java [80-84] -} catch (QueryJobNotFoundException e) { - listener.onFailure(new ResourceNotFoundException(e.getMessage())); -} catch (QueryJobForbiddenException e) { - listener.onFailure(new OpenSearchStatusException(e.getMessage(), RestStatus.FORBIDDEN)); +} catch (QueryJobNotFoundException | QueryJobForbiddenException e) { + listener.onFailure(translateToTransportException(e)); +} +private Exception translateToTransportException(Exception e) { + if (e instanceof QueryJobNotFoundException) { + return new ResourceNotFoundException(e.getMessage()); + } else if (e instanceof QueryJobForbiddenException) { + return new OpenSearchStatusException(e.getMessage(), RestStatus.FORBIDDEN); + } + return e; +} + Suggestion importance[1-10]: 5 __ Why: Valid refactoring suggestion to reduce duplication and improve maintainability by extracting exception translation into a helper method. However, the impact is moderate as the current approach is clear and only handles two exception types.	Low
	Add logging for eviction edge cases The eviction timer uses a WeakReference to avoid retaining deleted jobs, but this creates a race condition where the job might be garbage collected before the timer fires, causing silent eviction failures. Consider tracking deletion state explicitly or using a different mechanism to handle the delete-before-eviction scenario. opensearch/src/main/java/org/opensearch/sql/job/RetentionPolicy.java [54-65] private void scheduleEviction(QueryJob job, Duration ttl) { QueryJobId id = job.id(); WeakReference<QueryJob> retained = new WeakReference<>(job); try { threadPool.schedule( () -> { QueryJob current = retained.get(); - if (current != null && store.remove(id, current)) { - LOG.debug("Evicted terminal query job [{}] after TTL", id.encode()); + if (current != null) { + boolean removed = store.remove(id, current); + if (removed) { + LOG.debug("Evicted terminal query job [{}] after TTL", id.encode()); + } else { + LOG.trace("Job [{}] already removed before eviction", id.encode()); + } + } else { + LOG.trace("Job [{}] was garbage collected before eviction", id.encode()); } }, Suggestion importance[1-10]: 4 __ Why: The suggestion adds useful diagnostic logging for edge cases in the eviction mechanism. While helpful for debugging, the WeakReference behavior is intentional and the additional logging has limited practical impact.	Low
	Standardize cancel response format The method returns different response formats for PPL jobs (state name string) versus Spark queries (formatted message string), creating an inconsistent API contract. Consider standardizing the return format to always return a structured response or always return a simple string, rather than mixing both approaches based on query type. async-query-core/src/main/java/org/opensearch/sql/spark/asyncquery/AsyncQueryExecutorServiceImpl.java [184-187] Optional<QueryJobId> jobId = asJobId(queryId); if (jobId.isPresent() && queryJobService != null) { - return queryJobService.delete(jobId.get(), currentPrincipal()).state().name(); + QueryJobStatus status = queryJobService.delete(jobId.get(), currentPrincipal()); + return String.format("Deleted async query with id: %s, status: %s", queryId, status.state().name()); } Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies an inconsistency in return formats, but the proposed change would break the existing contract for PPL jobs which now return JSON status objects. The inconsistency is intentional per the PR design.	Low

---

Previous suggestions

Suggestions up to commit 6d653e9

Category	Suggestion	Impact
General	Add logging for garbage collected jobs The WeakReference prevents retaining deleted jobs until TTL, but if the job is garbage collected before eviction fires, the scheduled task silently does nothing. This could leave orphaned entries in the store if the store holds a strong reference. Verify that the store doesn't retain jobs independently, or add logging when retained.get() returns null to detect potential leaks. opensearch/src/main/java/org/opensearch/sql/job/RetentionPolicy.java [54-65] private void scheduleEviction(QueryJob job, Duration ttl) { QueryJobId id = job.id(); WeakReference<QueryJob> retained = new WeakReference<>(job); try { threadPool.schedule( () -> { QueryJob current = retained.get(); - if (current != null && store.remove(id, current)) { + if (current == null) { + LOG.trace("Job [{}] was garbage collected before eviction", id.encode()); + } else if (store.remove(id, current)) { LOG.debug("Evicted terminal query job [{}] after TTL", id.encode()); } }, TimeValue.timeValueMillis(ttl.toMillis()), GENERIC_POOL); Suggestion importance[1-10]: 5 __ Why: Adding trace-level logging when retained.get() returns null could help detect potential issues, though the WeakReference pattern is intentionally designed to allow garbage collection. The suggestion is valid but represents a minor observability improvement rather than a correctness issue.	Low
	Standardize response format across query types The response format differs based on whether the query ID is parsed (JSON for PPL, plain string for Spark). This inconsistency could confuse clients. Consider standardizing the response format to always return JSON, or clearly document this behavior difference in the API contract. async-query/src/main/java/org/opensearch/sql/spark/transport/TransportCancelAsyncQueryRequestAction.java [75-79] -listener.onResponse( - new CancelAsyncQueryActionResponse( - parsed.isPresent() - ? new JSONObject().put("status", result).toString() - : String.format("Deleted async query with id: %s", result))); +String responseBody = parsed.isPresent() + ? new JSONObject().put("status", result).toString() + : new JSONObject().put("message", String.format("Deleted async query with id: %s", result)).toString(); +listener.onResponse(new CancelAsyncQueryActionResponse(responseBody)); Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies an inconsistency in response formats between PPL (JSON with status) and Spark (plain string message). However, the PR intentionally maintains backward compatibility for Spark's existing contract while introducing the new JSON format for PPL, as documented in the tests.	Low
	Add logging for request type detection The routing logic determines both HTTP status and thread pool based on isPpl. If the PPL detection logic (isPplRequest) has a bug or edge case, both the status code and execution pool will be incorrect. Consider adding validation or logging to ensure isPpl is correctly determined, especially since this affects the API contract. async-query/src/main/java/org/opensearch/sql/spark/rest/RestAsyncQueryManagementAction.java [218-224] boolean isPpl = isPplRequest(restRequest); RestStatus successStatus = isPpl ? RestStatus.OK : RestStatus.NO_CONTENT; String poolName = isPpl ? ThreadPool.Names.GENERIC : SQL_WORKER_THREAD_POOL_NAME; +LOG.debug("DELETE request for queryId={} detected as isPpl={}, using pool={}", queryId, isPpl, poolName); Suggestion importance[1-10]: 3 __ Why: Adding debug logging for isPpl detection could aid troubleshooting, but the suggestion doesn't address a concrete bug. The isPplRequest method is already tested and the routing logic is validated through integration tests in RestAsyncQueryManagementActionTest.java.	Low

Suggestions up to commit 7d243cd

Category	Suggestion	Impact
General	Add timeout consideration for blocking cancellations The routing logic assumes PPL DELETE operations are always cheap and non-blocking, but if the job is running a complex query, the cancellation may involve waiting for the engine to reach a cancellation checkpoint. Consider adding a timeout or monitoring mechanism to ensure DELETE requests on the generic pool don't block indefinitely if cancellation takes longer than expected. async-query/src/main/java/org/opensearch/sql/spark/rest/RestAsyncQueryManagementAction.java [218-224] boolean isPpl = isPplRequest(restRequest); // PPL jobs acknowledge with their final status; Spark keeps its existing 204 contract. RestStatus successStatus = isPpl ? RestStatus.OK : RestStatus.NO_CONTENT; -// PPL DELETE is a store lookup and a cancel — cheap and non-blocking. Route it off sql-worker -// so a saturated query pool can't delay cancellations that are supposed to free that pool up. +// PPL DELETE is a store lookup and a cancel — typically cheap and non-blocking. +// Route it off sql-worker so a saturated query pool can't delay cancellations. // Spark DELETE still runs on sql-worker to preserve its existing dispatch contract. +// Note: If cancellation blocks, consider adding timeout handling. String poolName = isPpl ? ThreadPool.Names.GENERIC : SQL_WORKER_THREAD_POOL_NAME; Suggestion importance[1-10]: 5 __ Why: The suggestion raises a valid concern about potential blocking during cancellation, but the comment change alone doesn't address the issue. The cancellation mechanism is cooperative and designed to be non-blocking at the transport layer. Adding a timeout would require architectural changes beyond a simple comment update, and the current design already handles this through the engine's cancellation checkpoints.	Low
	Avoid double-wrapping serializable exceptions The exception translation logic converts domain exceptions to transport-serializable exceptions, but the generic Exception catch-all may inadvertently wrap already-serializable exceptions. Consider checking if the exception is already an OpenSearchException before wrapping to avoid double-wrapping and preserve the original exception type. async-query/src/main/java/org/opensearch/sql/spark/transport/TransportCancelAsyncQueryRequestAction.java [80-86] } catch (QueryJobNotFoundException e) { listener.onFailure(new ResourceNotFoundException(e.getMessage())); } catch (QueryJobForbiddenException e) { listener.onFailure(new OpenSearchStatusException(e.getMessage(), RestStatus.FORBIDDEN)); } catch (Exception e) { - listener.onFailure(e); + if (e instanceof org.opensearch.OpenSearchException) { + listener.onFailure(e); + } else { + listener.onFailure(new OpenSearchException(e)); + } } Suggestion importance[1-10]: 4 __ Why: The suggestion raises a valid concern about potentially double-wrapping OpenSearchException instances, but the current code already handles the most critical cases (QueryJobNotFoundException and QueryJobForbiddenException). The improvement would be marginal and the suggested code introduces a new dependency without clear evidence of the problem occurring.	Low
	Standardize return format across job types The method returns different response types for PPL jobs (state name string) versus Spark jobs (formatted message string), creating an inconsistent API contract. Consider standardizing the return format or documenting this behavioral difference clearly in the method's Javadoc to prevent confusion for callers. async-query-core/src/main/java/org/opensearch/sql/spark/asyncquery/AsyncQueryExecutorServiceImpl.java [184-187] Optional<QueryJobId> jobId = asJobId(queryId); if (jobId.isPresent() && queryJobService != null) { - return queryJobService.delete(jobId.get(), currentPrincipal()).state().name(); + QueryJobStatus status = queryJobService.delete(jobId.get(), currentPrincipal()); + // Return consistent format: state name for PPL jobs + return status.state().name(); } Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies that the method returns different formats for PPL (state name) vs Spark (formatted message), but this is intentional design as documented in the method's Javadoc. The inconsistency is by design to maintain backward compatibility with Spark's existing contract while providing structured responses for PPL jobs.	Low
	Prevent indirect job retention via captured references The WeakReference prevents the eviction timer from retaining deleted jobs, but the lambda captures id which may indirectly hold a strong reference to the job through the store's internal map. Verify that capturing id alone doesn't prevent garbage collection of the job when it's deleted before TTL expiration. opensearch/src/main/java/org/opensearch/sql/job/RetentionPolicy.java [54-65] private void scheduleEviction(QueryJob job, Duration ttl) { QueryJobId id = job.id(); + String encodedId = id.encode(); // The eviction timer must not retain a deleted job until TTL. WeakReference<QueryJob> retained = new WeakReference<>(job); try { threadPool.schedule( () -> { - QueryJobStatus current = retained.get(); + QueryJob current = retained.get(); if (current != null && store.remove(id, current)) { - LOG.debug("Evicted terminal query job [{}] after TTL", id.encode()); + LOG.debug("Evicted terminal query job [{}] after TTL", encodedId); } }, Suggestion importance[1-10]: 2 __ Why: The suggestion misunderstands the purpose of the WeakReference. The QueryJobId is a small value object that doesn't hold a reference to the job itself. Capturing id in the lambda is necessary for the removal operation and doesn't prevent garbage collection of the job. The suggested change of capturing encodedId instead provides no benefit.	Low

Suggestions up to commit 163015d

Category	Suggestion	Impact
General	Handle forwarding exceptions consistently The forwarding logic wraps AsyncQueryNotFoundException into ResourceNotFoundException, but does not handle QueryJobForbiddenException or QueryJobNotFoundException that may be thrown during forwarding. These exceptions should also be translated to appropriate HTTP status codes to maintain consistent error responses across forwarding boundaries. async-query/src/main/java/org/opensearch/sql/spark/transport/TransportCancelAsyncQueryRequestAction.java [62-78] Optional<QueryJobId> parsed = QueryJobId.tryParse(queryId); if (parsed.isPresent() && !clusterService.localNode().getId().equals(parsed.get().ownerNodeId())) { AsyncQueryOwnerRouting.forwardToOwner( clusterService, transportService, parsed.get(), NAME, request, CancelAsyncQueryActionResponse::new, ActionListener.wrap( listener::onResponse, - failure -> - listener.onFailure( - failure instanceof AsyncQueryNotFoundException - ? new ResourceNotFoundException(failure.getMessage()) - : failure))); + failure -> { + if (failure instanceof AsyncQueryNotFoundException + || failure instanceof QueryJobNotFoundException) { + listener.onFailure(new ResourceNotFoundException(failure.getMessage())); + } else if (failure instanceof QueryJobForbiddenException) { + listener.onFailure(new OpenSearchStatusException(failure.getMessage(), RestStatus.FORBIDDEN)); + } else { + listener.onFailure(failure); + } + })); return; } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that QueryJobForbiddenException and QueryJobNotFoundException should be handled during forwarding. However, the code already handles these exceptions at lines 87-90 in the local execution path, and the forwarding wrapper at line 76 already translates AsyncQueryNotFoundException to ResourceNotFoundException. The suggestion would add redundant handling since these exceptions are already caught and translated in the local execution block.	Medium
	Prevent premature job garbage collection The WeakReference approach may allow premature garbage collection of the job before TTL expires if no other references exist, potentially preventing proper eviction logging and cleanup. Consider using a strong reference with explicit cleanup or verify that the store maintains a reference during the TTL period. opensearch/src/main/java/org/opensearch/sql/job/RetentionPolicy.java [54-68] private void scheduleEviction(QueryJob job, Duration ttl) { QueryJobId id = job.id(); - // The eviction timer must not retain a deleted job until TTL. - WeakReference<QueryJob> retained = new WeakReference<>(job); try { threadPool.schedule( () -> { - QueryJob current = retained.get(); + QueryJob current = store.find(id).orElse(null); if (current != null && store.remove(id, current)) { LOG.debug("Evicted terminal query job [{}] after TTL", id.encode()); } }, TimeValue.timeValueMillis(ttl.toMillis()), GENERIC_POOL); Suggestion importance[1-10]: 3 __ Why: The suggestion misunderstands the intent of the WeakReference. The PR explicitly uses WeakReference to avoid retaining deleted jobs until TTL (as stated in the comment). The store maintains the strong reference during TTL, and the WeakReference allows early cleanup if the job is deleted. The suggested change would reintroduce the memory retention issue the PR is trying to fix.	Low

Suggestions up to commit 57ed0bc

Category	Suggestion	Impact
General	Standardize delete response format The response format differs based on whether the query ID is parsed, creating inconsistent API behavior. For PPL jobs, a JSON object with status is returned, while for Spark queries, a plain string message is returned. This inconsistency may confuse API consumers and complicate client-side response handling. async-query/src/main/java/org/opensearch/sql/spark/transport/TransportCancelAsyncQueryRequestAction.java [80-86] String result = asyncQueryExecutorService.cancelQuery(queryId, new NullAsyncQueryRequestContext()); -listener.onResponse( - new CancelAsyncQueryActionResponse( - parsed.isPresent() - ? new JSONObject().put("status", result).toString() - : String.format("Deleted async query with id: %s", result))); +String responseBody = parsed.isPresent() + ? new JSONObject().put("status", result).toString() + : new JSONObject().put("message", String.format("Deleted async query with id: %s", result)).toString(); +listener.onResponse(new CancelAsyncQueryActionResponse(responseBody)); Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies an inconsistency in response formats between PPL and Spark queries. However, the PR intentionally maintains this difference to preserve backward compatibility with Spark's existing contract (204 No Content vs 200 OK). The suggestion would break existing Spark clients.	Low

Suggestions up to commit 893a012

Category	Suggestion	Impact
General	Standardize delete response format The response format differs between PPL jobs (JSON with status) and Spark queries (plain text message). This inconsistency may confuse API consumers. Consider standardizing the response format to always return JSON, or document this behavior clearly in the API contract. async-query/src/main/java/org/opensearch/sql/spark/transport/TransportCancelAsyncQueryRequestAction.java [80-86] String result = asyncQueryExecutorService.cancelQuery(queryId, new NullAsyncQueryRequestContext()); -listener.onResponse( - new CancelAsyncQueryActionResponse( - parsed.isPresent() - ? new JSONObject().put("status", result).toString() - : String.format("Deleted async query with id: %s", result))); +String responseBody = parsed.isPresent() + ? new JSONObject().put("status", result).toString() + : new JSONObject().put("message", String.format("Deleted async query with id: %s", result)).toString(); +listener.onResponse(new CancelAsyncQueryActionResponse(responseBody)); Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies an inconsistency in response formats between PPL jobs (JSON) and Spark queries (plain text). However, the PR intentionally maintains different response formats for backward compatibility with Spark's existing 204 contract, as evidenced by the RestAsyncQueryManagementAction changes. The suggestion's impact is moderate since standardization would be beneficial but conflicts with the PR's design decision.	Low
	Clarify queryId nullability handling The constructor reads queryId as optional, but the field is not marked as @Nullable and the class doesn't handle null values explicitly. If queryId can be null, add validation or null-safety annotations. If it must be non-null, use readString() instead of readOptionalString(). async-query/src/main/java/org/opensearch/sql/spark/transport/model/CancelAsyncQueryActionRequest.java [23-26] public CancelAsyncQueryActionRequest(StreamInput in) throws IOException { super(in); - this.queryId = in.readOptionalString(); + this.queryId = in.readString(); } Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid concern about queryId nullability. However, the use of readOptionalString() and writeOptionalString() is consistent with the serialization pattern, and the test file shows both null and non-null cases are handled. The suggestion to use readString() would break the null case handling shown in tests, making this a minor improvement suggestion rather than a bug fix.	Low
	Add logging for early GC The WeakReference prevents retaining a deleted job until TTL, but the eviction task still runs even if the job was already deleted. Consider checking if the job exists in the store before attempting removal to avoid unnecessary work, or add logging when the weak reference returns null to track early garbage collection. opensearch/src/main/java/org/opensearch/sql/job/RetentionPolicy.java [54-67] private void scheduleEviction(QueryJob job, Duration ttl) { QueryJobId id = job.id(); WeakReference<QueryJob> retained = new WeakReference<>(job); try { threadPool.schedule( () -> { QueryJob current = retained.get(); if (current != null && store.remove(id, current)) { LOG.debug("Evicted terminal query job [{}] after TTL", id.encode()); + } else if (current == null) { + LOG.trace("Query job [{}] was garbage collected before TTL eviction", id.encode()); } }, TimeValue.timeValueMillis(ttl.toMillis()), GENERIC_POOL); Suggestion importance[1-10]: 2 __ Why: The suggestion proposes adding trace logging when a job is garbage collected before TTL eviction. While this could provide observability, it's a minor enhancement with limited practical value since early GC is expected behavior when jobs are deleted. The existing code already handles this case correctly by checking if current != null.	Low

[github-actions]

Persistent review updated to latest commit ff7e4e8

[github-actions]

Persistent review updated to latest commit aa9f803

[codecov]

Codecov Report

❌ Patch coverage is 98.18182% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.87%. Comparing base (c640cb9) to head (f45c375).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
...n/java/org/opensearch/sql/job/RetentionPolicy.java	80.00%	0 Missing and 1 partial ⚠️

❌ Your project check has failed because the head coverage (63.87%) 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    #5846      +/-   ##
============================================
+ Coverage     63.64%   63.87%   +0.23%     
- Complexity     9017     9091      +74     
============================================
  Files           958      962       +4     
  Lines         40806    41033     +227     
  Branches       4632     4668      +36     
============================================
+ Hits          25971    26211     +240     
+ Misses        13964    13944      -20     
- Partials        871      878       +7     

Flag	Coverage Δ
sql-engine	63.87% <98.18%> (+0.23%)	⬆️

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.

[github-actions]

Persistent review updated to latest commit 03d00bb

[github-actions]

Persistent review updated to latest commit 893a012

[github-actions]

Persistent review updated to latest commit 57ed0bc

[github-actions]

Persistent review updated to latest commit 163015d

[RyanL1997]

LGTM - Tested this locally and it looks good to me. DELETE stopped queued and running jobs and freed their memory.

While testing I found a separate bug and opened an issue for it: #5860. A query rejected with Insufficient resources to start query leaves its PIT open until the keep-alive expires. It's pre-existing on main and not caused by this PR.

Mentioning it here because async PPL makes it easier to hit: one client can queue many queries and push the node past the memory limit.

[github-actions]

Persistent review updated to latest commit 7d243cd

[github-actions]

Persistent review updated to latest commit 6d653e9

[github-actions]

Persistent review updated to latest commit f45c375