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] Add Redis sink #3

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

Conversation

eskabetxe
Copy link
Member

@eskabetxe eskabetxe commented Jul 29, 2022

Add basic RedisSink based on AsyncSinkWriter

As Jedis dont have a async call, it was implemented.
Maybe better choice was to use lettuce or Redisson libs..

Need to be added a lot of commands, its basic to allow the discussion of implementation

@knaufk could you check?

@MartijnVisser
Copy link
Contributor

@eskabetxe Thanks a lot for the PR! It might make more sense for @sazzad16 and/or @chayim to review, as they are more familiar with Redis.

If you rebase this PR, you should also get the CI build information so we can validate that it works as expected.

Last but not least: before we merge it, we should get the FLIP accepted. There's currently a draft FLIP open (see https://cwiki.apache.org/confluence/display/FLINK/FLIP-254%3A+Redis+Streams+Connector) and I'm waiting for feedback from @sazzad16 before we can bring it up for discussion and vote in the Dev mailing list. If you have any feedback on the FLIP, feel free to let me know too!

@eskabetxe
Copy link
Member Author

Hi @MartijnVisser rebase done..
I will check the FLIP

@sazzad16
Copy link

Hi @eskabetxe, nice to meet you.

I see that the user would have to set (and so know and understand) exact Redis command which I feel not very intuitive from user's perspective.

Did you get to look at #2? It is based on Flink paradigm from some time ago but supports all of Sink, Source, TableSink, TableSource. It starts with Redis Streams data structure which is a great fit for Flink. Not to mention other data structures can also be supported later.

Let me know what do you think about #2. Perhaps we can collaborate to update it to current Flink paradigm?

@sazzad16
Copy link

@MartijnVisser The FLIP is great!

@eskabetxe
Copy link
Member Author

Hi @sazzad16,
Im not an expert on Redis Streams, but I think bot ways must be implemented..
For my knowledge, Redis Streams is used like (sorry for the analogy) if you are using Kafka no?

If you already have a Redis cluster using other commands like string, hash, list you cant migrate to streams without migration?
This is my use case, using Redis as a cache, I doubt I could change to stream for example.

I think for example in Sink, the use of stream or other commands could be implemented with config, what will change is SinkWriter implementation..

I would be happy to talk to you about it.

@sazzad16
Copy link

sazzad16 commented Aug 1, 2022

@eskabetxe Let's discuss.

Would Slack be a good place for this? Or must we use mailing list?

@MartijnVisser

@MartijnVisser
Copy link
Contributor

Would Slack be a good place for this? Or must we use mailing list?

I think you can DM each other in the Flink Slack workspace to collaborate on this topic. If you end up that you can't agree on the correct implementation strategy, then it should be brought up for a discussion on the Dev mailing list (or reach out to the User mailing list if you're looking for feedback).

@MartijnVisser
Copy link
Contributor

@eskabetxe @sazzad16 Is there any update on this process? Also check out https://cwiki.apache.org/confluence/display/FLINK/Externalized+Connector+development for some more insights on how to develop an externalized connector

@eskabetxe
Copy link
Member Author

Hi @MartijnVisser, it's pretty much at a standstill.
As it is only for streams it does not fit in our use case and it is difficult to dedicate time to it.
Anyway, let's see if we can get some time so we can move forward with it since it's practically all programmed.

@eskabetxe @sazzad16 Is there any update on this process? Also check out https://cwiki.apache.org/confluence/display/FLINK/Externalized+Connector+development for some more insights on how to develop an externalized connector

@eskabetxe
Copy link
Member Author

Hi @MartijnVisser , @sazzad16
I update the code to only have Redis Streams with the format of flink-connectors
if you could check this, then we move to other things like source, table etc

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