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

Ignore errors when Sink or Extractor is forcibly canceled #1992

Merged

Conversation

wangch079
Copy link
Member

Closes #1985

What's the change

  1. Changed the log level to warning when a sync job is forcibly canceled
  2. Ignore any error thrown in Sink or Extractor class if the Sink or Extractor object is forcibly canceled

Why the change

In the last release, we Forcibly cancel a sync job after waiting for 5 seconds. This should not be treated as ERROR, therefore we want to log it at warning level.

When we forcibly cancel the Sink or Extractor task, it will throw a ForceCanceledError. But sometimes there can be other errors thrown due to task cancellation, and be caught first. Before we log those errors as critical, we want to check if the task is forcibly canceled. If so, we will ignore those errors, and instead log a warning message that the task is forcibly canceled.

Checklists

Pre-Review Checklist

  • this PR has a meaningful title
  • this PR links to all relevant github issues that it fixes or partially addresses
  • this PR has a thorough description
  • Covered the changes with automated tests
  • Tested the changes locally
  • Added a label for each target release version (example: v7.13.2, v7.14.0, v8.0.0)

self._logger.warning(
f"Sink did not stop within {CANCELATION_TIMEOUT} seconds of cancelation, force-canceling the task."
)
return
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to re-raise here, if error is not ForceCancelledError or self._canceled

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch

@wangch079 wangch079 requested review from artem-shelkovnikov and a team December 19, 2023 14:10
Copy link
Member

@seanstory seanstory left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix!

@wangch079 wangch079 enabled auto-merge (squash) December 19, 2023 17:11
@wangch079 wangch079 merged commit 0c54116 into main Dec 19, 2023
@wangch079 wangch079 deleted the chenhui/ignore-error-if-sink-extractor-is-forcibly-canceled branch December 19, 2023 17:13
Copy link

💚 Backport PR(s) successfully created

Status Branch Result
8.12 #1996

This backport PR will be merged automatically after passing CI.

wangch079 added a commit that referenced this pull request Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error log on forced job cancelation
3 participants