-
Notifications
You must be signed in to change notification settings - Fork 4
[FLINK-15571][connector][WIP] Redis Stream connector for Flink #2
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
base: main
Are you sure you want to change the base?
Conversation
@MartijnVisser Pinging here - as this superceeds apache/flink#15487 |
@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! |
@MartijnVisser Thank you :) |
@MartijnVisser any luck finding someone to help? Hope you're enjoying your time off! |
@chayim Not yet, I'm bringing this PR up in our release meet-up which is scheduled for next week on Tuesday! |
@sazzad16 One request from my end: can you rebase on the current |
@MartijnVisser It is already rebased. Just rechecked. |
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.
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. |
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. |
Mostly brought from apache/flink#15487
Hi @sazzad16 , is there this going to support Redis Cluster? |
@Amitgb14 Well, that was the initial plan. |
No description provided.