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

Improve processor management and error handling #8657

Closed
hicqu opened this issue Mar 23, 2023 · 7 comments
Closed

Improve processor management and error handling #8657

hicqu opened this issue Mar 23, 2023 · 7 comments
Assignees
Labels
affects-7.1 area/ticdc Issues or PRs related to TiCDC. type/enhancement The issue or PR belongs to an enhancement.

Comments

@hicqu
Copy link
Contributor

hicqu commented Mar 23, 2023

Is your feature request related to a problem?

  • If sink error occurs, it's better to let puller and redo keep running;
  • If puller error occurs, it's better to let sink and redo keep running;
    • although generally puller doesn't emit errors, seems the only error can be "etcd context canceled"
  • If redo error occurs, it's better to let sink keep running;
  • If etcd session is broken, processor.Close should be called correctly, or some stale context will be left. For example, SortEngine instances.
  • Many sub components of Processor use errCh directly. When pushing an error into the channel, some different styles are used:
    • with a default branch in select; so the error can be dropped silenctly;
    • with a case ctx.Done() branch in select, which means the processor has been closed correctly.
    • Personally I prefer the second style, but code needs to be simplified.

Describe the feature you'd like

I want to

  • Seperate Processor lifetime from etcd session. Processor can only be closed explicitly instead of canceling the etcd session's context;
  • Ensure Processor.Close will always be called correctly when destroying a Processor instance;
  • Unify and simplify error handling in all sub components of Processor;
  • Isolate error handling of sink, puller and redo, so that one component failure won't break whole system.

Describe alternatives you've considered

No response

Teachability, Documentation, Adoption, Migration Strategy

No response

@nongfushanquan
Copy link
Contributor

nongfushanquan commented May 16, 2023

/label affects-7.1

@nongfushanquan
Copy link
Contributor

/remove-type feature
/type enhancement

@ti-chi-bot ti-chi-bot bot added type/enhancement The issue or PR belongs to an enhancement. and removed type/feature Issues about a new feature labels May 16, 2023
@nongfushanquan
Copy link
Contributor

/area ticdc

@nongfushanquan
Copy link
Contributor

/label affects-7.1

@nongfushanquan
Copy link
Contributor

/remove-label affects-6.5

@ti-chi-bot ti-chi-bot bot removed the affects-6.5 label May 30, 2023
hicqu added a commit to ti-chi-bot/tiflow that referenced this issue May 30, 2023
hicqu added a commit to ti-chi-bot/tiflow that referenced this issue May 30, 2023
hicqu added a commit to ti-chi-bot/tiflow that referenced this issue May 30, 2023
@nongfushanquan
Copy link
Contributor

/close

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jun 12, 2023

@nongfushanquan: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ti-chi-bot ti-chi-bot bot closed this as completed Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-7.1 area/ticdc Issues or PRs related to TiCDC. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

3 participants