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-33461][Connector/JDBC] Support streaming related semantics for the new JDBC source #119

Merged
merged 2 commits into from
Jun 14, 2024

Conversation

RocMarshal
Copy link
Contributor

@RocMarshal RocMarshal commented May 10, 2024

  • Introduce getLatestOptionalState & setOptionalState for JdbcParameterValuesProvider to process provider state.
  • Introduce SlideTimingParamameterProvider for supporting continuous splits
  • Introduce streaming semantic based on ResultSet.TYPE_SCROLL_INSENSITIVE
  • Introduce JdbcSourceStreamRelatedITCase for testing.

@RocMarshal RocMarshal force-pushed the FLINK-33461 branch 2 times, most recently from a2f6633 to a2fb896 Compare May 10, 2024 15:51
@RocMarshal RocMarshal closed this May 10, 2024
@RocMarshal RocMarshal reopened this May 10, 2024
@RocMarshal RocMarshal changed the title Flink 33461 [FLINK-33461][Connector/JDBC] Support streaming related semantics for the new jdbc source May 11, 2024
@RocMarshal RocMarshal changed the title [FLINK-33461][Connector/JDBC] Support streaming related semantics for the new jdbc source [FLINK-33461][Connector/JDBC] Support streaming related semantics for the new JDBC source May 11, 2024
@RocMarshal RocMarshal marked this pull request as ready for review May 17, 2024 11:10
@RocMarshal RocMarshal force-pushed the FLINK-33461 branch 6 times, most recently from 8f661db to e14152d Compare May 21, 2024 15:27
@RocMarshal
Copy link
Contributor Author

RocMarshal commented May 22, 2024

Hi, @eskabetxe @leonardBang @Jiabao-Sun Could you help have a look if you had the free time ?
Thank you.

Copy link
Member

@eskabetxe eskabetxe left a comment

Choose a reason for hiding this comment

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

LGTM

@RocMarshal RocMarshal force-pushed the FLINK-33461 branch 3 times, most recently from 5fd7b4a to 83365e3 Compare June 2, 2024 11:43
@1996fanrui 1996fanrui self-assigned this Jun 3, 2024
@RocMarshal RocMarshal force-pushed the FLINK-33461 branch 5 times, most recently from 1f6a037 to 4b056a6 Compare June 6, 2024 03:08

## `JdbcSink.sink`
## Source of JDBC Connector(Experimental)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## Source of JDBC Connector(Experimental)
## Source of JDBC Connector(Experimental)

This part is not needed.

Configuration goes as follow (see also {{< javadoc file="org/apache/flink/connector/jdbc/source/JdbcSource.html" name="JdbcSource javadoc" >}}
and {{< javadoc file="org/apache/flink/connector/jdbc/source/JdbcSourceBuilder.html" name="JdbcSourceBuilder javadoc" >}}).

### `JdbcSource.builder`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### `JdbcSource.builder`
## Jdbc Source

Jdbc Source can be as the second level instead of the third level. You can refer Kafka connector doc[1]. Kafka Source, Kafka SourceFunction and Kafka Producer as the second level. We don't need to distinguish the source or sink. (Current level is a little fine-grained.)

[1] https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/kafka/#kafka-source

```

#### Minimalist Streaming Semantic and ContinuousUnBoundingSettings
If you want to generate continuous milliseconds parameters based on sliding-window,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If you want to generate continuous milliseconds parameters based on sliding-window,
If you want to generate continuous milliseconds parameters based on sliding-window,

Add the empty line.

@RocMarshal RocMarshal force-pushed the FLINK-33461 branch 2 times, most recently from 3508075 to 78efce0 Compare June 6, 2024 08:23
Copy link
Contributor Author

@RocMarshal RocMarshal left a comment

Choose a reason for hiding this comment

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

Thanks @1996fanrui for the review.
I updated based on your comments.
PTAL if you had the free time~
CC @eskabetxe

Copy link
Member

@1996fanrui 1996fanrui left a comment

Choose a reason for hiding this comment

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

Thanks @RocMarshal for the hard work, and thanks @eskabetxe for the review!

LGTM, I will merge it next Tuesday(in 4 days) if there are no objections.

@1996fanrui 1996fanrui merged commit 1bab533 into apache:main Jun 14, 2024
12 checks passed
@RocMarshal
Copy link
Contributor Author

👍 Thank you @1996fanrui @eskabetxe very much for your review and attention for the feature !

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.

3 participants