Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clp-package: Fix several search job cancellation issues: #425

Merged
merged 2 commits into from
Jun 6, 2024

Conversation

wraymo
Copy link
Contributor

@wraymo wraymo commented Jun 5, 2024

References

Description

This PR addresses several cancellation issues:

  • Previously, when all search tasks completed, sending a cancellation request changed the status to CANCELLING. Since the job was already removed from active_jobs, it remained in an intermediate state. This PR ensures that a cancellation request is only sent if the job status is PENDING or RUNNING. Besides, for completed jobs in RUNNING status, even if the cancellation request was sent, the job status is updated to either SUCCEEDED or FAILED based on the actual completion.
  • The wrong type was passed when handling job cancellation, so that the code never reached cancel_job_except_reducer. This PR fixes this issue.

Validation performed

  • Started the package and compressed the hive-24hrs dataset.
  • In the command line, executed sbin/search.sh "org.apache.hadoop.metrics2.impl.MetricsConfig: loaded properties from hadoop-metrics2.properties" and checked search_jobs table in the database.
  • Ran the same query, and cancelled it before it finished. Checked search_jobs table in the database.

@kirkrodrigues kirkrodrigues self-requested a review June 5, 2024 19:33
Copy link
Member

@kirkrodrigues kirkrodrigues left a comment

Choose a reason for hiding this comment

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

Nice catches! A few minor suggestions.

Comment on lines 93 to 94
WHERE ${SEARCH_JOBS_TABLE_COLUMN_NAMES.ID} = ?
AND ${SEARCH_JOBS_TABLE_COLUMN_NAMES.STATUS}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
WHERE ${SEARCH_JOBS_TABLE_COLUMN_NAMES.ID} = ?
AND ${SEARCH_JOBS_TABLE_COLUMN_NAMES.STATUS}
WHERE ${SEARCH_JOBS_TABLE_COLUMN_NAMES.ID} = ?
AND ${SEARCH_JOBS_TABLE_COLUMN_NAMES.STATUS}

@wraymo wraymo requested a review from kirkrodrigues June 5, 2024 20:28
Copy link
Member

@kirkrodrigues kirkrodrigues left a comment

Choose a reason for hiding this comment

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

For the PR title/commit description, how about:

clp-package: Fix several search job cancellation issues:
- Only allow cancelling pending or running jobs from the webui.
- Set a job's status completion status regardless of whether it was concurrently cancelled.
- Cast search job ID appropriately before indexing the in-memory active jobs dict.

@wraymo wraymo changed the title clp-package: Fix several issues with search job cancellation clp-package: Fix several search job cancellation issues Jun 6, 2024
@wraymo wraymo merged commit 2c37cc6 into y-scope:main Jun 6, 2024
2 checks passed
@kirkrodrigues kirkrodrigues changed the title clp-package: Fix several search job cancellation issues clp-package: Fix several search job cancellation issues: Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants