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] Redis Stream connector for Flink #15487

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

sazzad16
Copy link

@sazzad16 sazzad16 commented Apr 5, 2021

What is the purpose of the change

Introduce a new Flink connector with Redis Stream data structure.

Brief change log

  • Implement a new Flink connector with Redis Stream.

Verifying this change

This change added tests and can be verified as follows:

  • Added basic tests

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): yes
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: don't know
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? yes
  • If yes, how is the feature documented? not documented

@flinkbot
Copy link
Collaborator

flinkbot commented Apr 5, 2021

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit 5f3ad56 (Fri May 28 11:06:32 UTC 2021)

Warnings:

  • 2 pom.xml files were touched: Check for build and licensing issues.
  • No documentation files were touched! Remember to keep the Flink docs up to date!
  • This pull request references an unassigned Jira ticket. According to the code contribution guide, tickets need to be assigned before starting with the implementation work.

Mention the bot in a comment to re-run the automated checks.

Review Progress

  • ❓ 1. The [description] looks good.
  • ❓ 2. There is [consensus] that the contribution should go into to Flink.
  • ❓ 3. Needs [attention] from.
  • ❓ 4. The change fits into the overall [architecture].
  • ❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.


The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commands
The @flinkbot bot supports the following commands:

  • @flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
  • @flinkbot approve all to approve all aspects
  • @flinkbot approve-until architecture to approve everything until architecture
  • @flinkbot attention @username1 [@username2 ..] to require somebody's attention
  • @flinkbot disapprove architecture to remove an approval you gave earlier

@flinkbot
Copy link
Collaborator

flinkbot commented Apr 5, 2021

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@sazzad16
Copy link
Author

sazzad16 commented Apr 5, 2021

@rmetzger How to improve from review=[description]? Is there anything I can do from my side?

PS: I plan to add documentation after the implementation is finalized.

Thank you.

@KarmaGYZ
Copy link
Contributor

KarmaGYZ commented Apr 6, 2021

Hi, @sazzad16 . Thanks for opening the PR. However, as the Automated Checks describe, we should first reach a consensus in JIRA ticket and then push the PR. I think you can:

  1. Open a discussion thread in user/dev ML on whether we need this connector.
  2. Attach a design doc of this connector to the ticket.

@sazzad16
Copy link
Author

sazzad16 commented Apr 6, 2021

@KarmaGYZ Thank you for your response.

Briefly speaking, this is a simple idea. i) Save data into Redis. ii) Get data from Redis. So I'm not sure about what to do here.

Could you please refer to an example (design doc)? Thanks again!

@KarmaGYZ
Copy link
Contributor

KarmaGYZ commented Apr 6, 2021

@sazzad16 Maybe you can refer to this FLIP. All in all, I think we should reach a consensus on whether we need this connector first. Just like Yun Tang's comment on FLINK-15571.

@sazzad16
Copy link
Author

sazzad16 commented Apr 6, 2021

@KarmaGYZ It'll be helpful. Thank you.

@sazzad16 sazzad16 changed the title [FLINK-15571][connector] [POC] Redis Stream connector for Flink [FLINK-15571][connector] Redis Stream connector for Flink Apr 21, 2021
@gkorland
Copy link

@KarmaGYZ how one can add a new FLIP?

@MartijnVisser
Copy link
Contributor

@sazzad16 @gkorland Hi! Sorry for the late reply. Are you still interested in contributing/maintaining a Redis connector for Flink?

@sazzad16
Copy link
Author

sazzad16 commented Apr 7, 2022

@MartijnVisser We'd love to. Let us know how to proceed.

@MartijnVisser
Copy link
Contributor

@sazzad16 We're currently in the process of externalizing connectors from Flink's main repository to their own individual repositories. For example, the Elasticsearch connector is being moved from this repo to https://github.com/apache/flink-connector-elasticsearch. While working on moving out Elasticsearch, we're also creating some templates/examples/archetype for all other connectors.

The idea would be that we create a new repo for Redis, flink-connector-redis. You could open up PRs against that repo. A Source should be build via the Source API (see FLIP-27 https://cwiki.apache.org/confluence/display/FLINK/FLIP-27%3A+Refactor+Source+Interface) and Sinks using the Unified Sink API / ASync API (see FLIP-143 https://cwiki.apache.org/confluence/display/FLINK/FLIP-143%3A+Unified+Sink+API and FLIP-171 https://cwiki.apache.org/confluence/display/FLINK/FLIP-171%3A+Async+Sink). This also depends on the level of guarantees (at least once, at most once, exactly once etc) you want to guarantee. There are maintainers in the Flink community who can help with reviews for these type of implementations.

@gkorland
Copy link

gkorland commented Apr 11, 2022

@MartijnVisser that sounds like a great plan, when do you plan to open the new repo?

(@chayim)

@MartijnVisser
Copy link
Contributor

@gkorland I'll open an announce thread on the Dev mailing list today and I'll find a PMC who can create a repo. Would take a couple of days at most

@MartijnVisser
Copy link
Contributor

@gkorland @chayim Apologies for the slight delay, but the repo is there. You can find it at https://github.com/apache/flink-connector-redis

The current best implementation can be found at https://github.com/apache/flink-connector-elasticsearch

Please ping me in case you have any questions or remarks

@chayim
Copy link

chayim commented May 4, 2022

Thanks @MartijnVisser. Appreciate it!

sazzad16 added a commit to sazzad16/flink-connector-redis-streams that referenced this pull request May 19, 2022
Mostly brought from apache/flink#15487
@sazzad16
Copy link
Author

@MartijnVisser Just submitted the PR apache/flink-connector-redis-streams#2 in the new repo. Please consider that as work in progress and it would be great to get some initial reviews.

sazzad16 added a commit to sazzad16/flink-connector-redis-streams that referenced this pull request Aug 9, 2022
Mostly brought from apache/flink#15487
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants