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

Add CDCClient to client-java with a few tiny changes for TiFlink #174

Merged
merged 5 commits into from
Jun 7, 2021

Conversation

shanzi
Copy link
Contributor

@shanzi shanzi commented May 25, 2021

This PR adds the pure java implementation of CDCClient which comunicates directly with TiKV with GRPC for change logs. It also includes some tiny changes to make TiFlink's implementation easier.
For details about TiFlink, please see this link.

@marsishandsome
Copy link
Collaborator

build failed

[2021-05-25T13:36:04.891Z] [ERROR] /home/jenkins/agent/git/client-java/src/main/java/org/tikv/cdc/RegionCDCClient.java:[27,10] cannot find symbol

[2021-05-25T13:36:04.891Z] [ERROR]   symbol:   method of(org.tikv.kvproto.Cdcpb.Event.LogType,org.tikv.kvproto.Cdcpb.Event.LogType,org.tikv.kvproto.Cdcpb.Event.LogType,org.tikv.kvproto.Cdcpb.Event.LogType)

[2021-05-25T13:36:04.891Z] [ERROR]   location: interface java.util.Set

@shanzi
Copy link
Contributor Author

shanzi commented May 31, 2021

@marsishandsome What's the desire JDK version for java-client now? It's a java11 method.

@marsishandsome
Copy link
Collaborator

@marsishandsome What's the desire JDK version for java-client now? It's a java11 method.

java8

@shanzi
Copy link
Contributor Author

shanzi commented Jun 2, 2021

@marsishandsome Updated to java8

@marsishandsome
Copy link
Collaborator

/run-all-tests

pom.xml Outdated Show resolved Hide resolved
Copy link
Collaborator

@marsishandsome marsishandsome left a comment

Choose a reason for hiding this comment

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

LGTM

@marsishandsome
Copy link
Collaborator

@birdstorm PTAL

Copy link
Collaborator

@birdstorm birdstorm left a comment

Choose a reason for hiding this comment

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

LGTM

@birdstorm
Copy link
Collaborator

One more question, @shanzi can you please describe how to use multiple CDCClient at the same time? e.g., are there any precautions, and what would happen if the key ranges collide?

@shanzi
Copy link
Contributor Author

shanzi commented Jun 4, 2021

@birdstorm one CDCClient can only accept one key range, when using multiple CDCClients at the same time, each one will just work independently. The user is responsible for assigning proper key ranges for each. Thus overlapping key range won't have any problem. In other words, if the user runs two clients with overlapping key range, there will be two RegionCDCClient to the same region/store (That's why we don't use any Channel cache).

@marsishandsome Can I merge this now? And what's the schedule to release 3.1.0?

Thanks.

@birdstorm
Copy link
Collaborator

@birdstorm one CDCClient can only accept one key range, when using multiple CDCClients at the same time, each one will just work independently. The user is responsible for assigning proper key ranges for each. Thus overlapping key range won't have any problem. In other words, if the user runs two clients with overlapping key range, there will be two RegionCDCClient to the same region/store (That's why we don't use any Channel cache).

@marsishandsome Can I merge this now? And what's the schedule to release 3.1.0?

Thanks.

Will two RegionCDCClient to the same region/store be a problem? That is what I am concerned about.

@shanzi
Copy link
Contributor Author

shanzi commented Jun 5, 2021

@birdstorm There will be two channels to the single store. So, I think there won't be any issue.

Signed-off-by: chase <yun.er.run@gmail.com>
Signed-off-by: chase <yun.er.run@gmail.com>
Signed-off-by: chase <yun.er.run@gmail.com>
Signed-off-by: chase <yun.er.run@gmail.com>
Signed-off-by: chase <yun.er.run@gmail.com>
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.

4 participants