Skip to content

SOLR-18248: Migration of CancelTask V2 API to JAX-RS construct - #4828

Open
jaykay12 wants to merge 83 commits into
apache:mainfrom
jaykay12:SOLR-18248-cancel-tasks
Open

SOLR-18248: Migration of CancelTask V2 API to JAX-RS construct#4828
jaykay12 wants to merge 83 commits into
apache:mainfrom
jaykay12:SOLR-18248-cancel-tasks

Conversation

@jaykay12

@jaykay12 jaykay12 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/SOLR-18248

Description

Migration of Cancel Task V2 API into JAX-RS Construct

Solution

No AI Usage.

Tests

Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide
  • I have added a changelog entry for my change

"/tasks/cancel": {
"class": "solr.QueryCancellationHandler",
"class": "solr.TaskCancellationHandler",
"useParams":"_TASK_CANCELLATION",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the fact that this was called _TASK__CONCELLATION already is some good evidence that our renaming is headed int he right direction!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, caan you look the components that are defined in this? I poked aroudn a bit and I think that the "components" defined in this file aren't actually needed anymore or used.... Both for the cancel handler and the list tasks handler below.. Can you investigate and see if this is old dead code, maybe from our previous work to migrate tasks? I don't think it breaks anything, but since we are in here, we should clean this up.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, majorly all cleanup has been done related with this migration alongside only, there looks like no other scope of cleanup here right now.

@epugh epugh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting closer! Some comments on two things to change.

"/tasks/cancel": {
"class": "solr.QueryCancellationHandler",
"class": "solr.TaskCancellationHandler",
"useParams":"_TASK_CANCELLATION",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, caan you look the components that are defined in this? I poked aroudn a bit and I think that the "components" defined in this file aren't actually needed anymore or used.... Both for the cancel handler and the list tasks handler below.. Can you investigate and see if this is old dead code, maybe from our previous work to migrate tasks? I don't think it breaks anything, but since we are in here, we should clean this up.

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Aug 30, 2026

@jaykay12 jaykay12 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self-reviewed ✅

@epugh epugh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@epugh
epugh requested review from gerlowskija and a balanced review from Copilot August 31, 2026 10:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates the V2 task-cancellation API to JAX-RS while retaining V1 compatibility and distributed cancellation.

Changes:

  • Adds the typed DELETE /tasks/{taskID} endpoint and response model.
  • Consolidates local and distributed task operations.
  • Updates registration, tests, documentation, and changelog.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
task-management.adoc Documents the V2 cancellation endpoint.
CancelTaskTest.java Tests local cancellation behavior.
SolrCoreTest.java Updates implicit-handler assertion.
ImplicitPlugins.json Registers the replacement handler.
TaskManagementHandler.java Removes obsolete component dispatch.
TaskCancellationHandler.java Preserves V1 and registers JAX-RS.
QueryCancellationComponent.java Removes legacy cancellation component.
ActiveTaskQuerySupport.java Adds shared distributed cancellation.
CancelTaskAPI.java Removes legacy annotated V2 API.
CancelTask.java Implements the JAX-RS resource.
CancelTaskResponse.java Adds the typed response model.
TasksApi.java Defines the DELETE endpoint contract.
SOLR-18248-cancel-tasks.yml Adds the changelog entry.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread solr/core/src/test/org/apache/solr/handler/admin/api/CancelTaskTest.java Outdated
Comment thread solr/core/src/java/org/apache/solr/handler/admin/api/CancelTask.java Outdated
@epugh

epugh commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Okay, I think this is finally ready. I DID end up looking across all the TaskManagement apis in this, I guess maybe should have pushed to get the CancelTask in, and then in a seperate PR... but ohwell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:api cat:search documentation Improvements or additions to documentation tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants