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

Keep track of resolved timestamps with a channel #74

Closed
wants to merge 1 commit into from
Closed

Keep track of resolved timestamps with a channel #74

wants to merge 1 commit into from

Conversation

suzaku
Copy link
Contributor

@suzaku suzaku commented Oct 28, 2019

What problem does this PR solve?

A channel is needed to notify the processor of new resolved ts.

What is changed and how it works?

Keep track of resolved ts with a channel.

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

@suzaku
Copy link
Contributor Author

suzaku commented Oct 28, 2019

/rebuild

@@ -127,5 +133,13 @@ func (p *Puller) GetResolvedTs() uint64 {
}

func (p *Puller) CollectRawTxns(ctx context.Context, outputFn func(context.Context, txn.RawTxn) error) error {
return txn.CollectRawTxns(ctx, p.buf.Get, outputFn, p.tsTracker)
forwardResolve := func(ctx2 context.Context, ts uint64) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

how about get resolvedTS in L120? and we needn't onResolved function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not the same. The L120 one is just a partial resolved ts, what we get here is the one determined by SpanFrontier which considers resolved ts from all regions.

@suzaku suzaku added the WIP label Oct 28, 2019
@suzaku suzaku closed this Oct 29, 2019
@suzaku suzaku deleted the puller-channels branch October 29, 2019 07:24
amyangfei added a commit to amyangfei/tiflow that referenced this pull request May 6, 2022
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