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

[FLINK-15571][connector][WIP] Redis Stream connector for Flink #2

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sazzad16
Copy link

No description provided.

@chayim
Copy link

chayim commented May 25, 2022

@MartijnVisser Pinging here - as this superceeds apache/flink#15487

@MartijnVisser
Copy link
Contributor

@chayim Thanks for that! I'm off for a little bit but I'll see if we can find anyone who can help with a review of this!

@sazzad16
Copy link
Author

@MartijnVisser Thank you :)

@chayim
Copy link

chayim commented Jun 7, 2022

@MartijnVisser any luck finding someone to help? Hope you're enjoying your time off!

@MartijnVisser
Copy link
Contributor

@chayim Not yet, I'm bringing this PR up in our release meet-up which is scheduled for next week on Tuesday!

@MartijnVisser
Copy link
Contributor

@sazzad16 One request from my end: can you rebase on the current main branch to get the correct ASF repository configuration in. It will avoid creating a lot of comments towards the Dev mailing list

@sazzad16
Copy link
Author

sazzad16 commented Jun 7, 2022

@MartijnVisser It is already rebased. Just rechecked.
Is there anything missing?

@knaufk
Copy link

knaufk commented Jul 20, 2022

Thanks @sazzad16 for you contribution and patience. I think the community would really benefit from a Redis Connector and I'll try to help you get this in.

The main challenge with the current PR is that it uses the old - about to be deprecated - source and sink APIs of Apache Flink (SourceFunction, SinkFunction).

Could you try migrating your implemention to these new APIs?

For the Source, https://nightlies.apache.org/flink/flink-docs-release-1.15/docs/dev/datastream/sources/ contains a description of the interfaces and there are already a few examples like Kafka or Pulsar.

For the sink, there is ElasticSearch that uses the new Sink API as well as the FileSink. However, I would recommend you have a look whether you can leverage the Async Sink like e.g. the DynamoDB Sink is doing, which is also under development right now. As prerequisite, you would need to be able to asynchronously write to Redis and tell based on the resulting future whether the request was successful or not (see Public Interfaces in the Async Sink FLIP). From what I know about Redis this should be possible and would greatly simplify the implementation of a Sink.

Lastly, I propose we split this contribution up into at least four separate PRs to get this moving more quickly.

  • the Source
  • the Sink
  • the TableSource
  • the TableSink

Just start with what seems most relevant to you.

I am sorry about these requests to port the implementation to the new APIs, but building on the old APIs will not be sustainable and with the new APIs we immediately get all the support for both batch and stream execution in the DataStream and Table API as well as better chances this connector ties in perfectly with checkpointing and watermarking without much work from your side.

Thanks again and looking forward to hearing from you.

@sazzad16
Copy link
Author

Hi @knaufk,

Thanks for the pointers and the feedback!

@MartijnVisser has also sent me more read material including https://cwiki.apache.org/confluence/display/FLINK/FLIP+Connector+Template, https://cwiki.apache.org/confluence/display/FLINK/FLIP-252%3A+Amazon+DynamoDB+Sink+Connector, https://cwiki.apache.org/confluence/display/FLINK/FLIP-243%3A+Dedicated+Opensearch+connectors.

I'll read through the links, and figure out how to best change my code. This may lead to a single PR or multiple PRs, as I get down that path.

@Amitgb14
Copy link

Hi @sazzad16 , is there this going to support Redis Cluster?

@sazzad16
Copy link
Author

@Amitgb14 Well, that was the initial plan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants