-
Notifications
You must be signed in to change notification settings - Fork 6
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
Switch to sink interfaces #38
Conversation
snuyanzin
commented
Aug 11, 2023
•
edited
Loading
edited
- It switches to sink interfaces
- Also it generalises retry for both at least once and exactly once
- Extracts Flow control settings into BQ connector config options
- Extracts retry count into BQ connector option
public static final ConfigOption<Long> MAX_OUTSTANDING_REQUEST_BYTES = | ||
ConfigOptions.key("max-outstanding-request-bytes") | ||
.longType() | ||
.defaultValue(100 * 1024 * 1024L) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100Mb is a default value from BQ storage api
new EnumMap<>(LogicalTypeRoot.class); | ||
@PublicEvolving | ||
public class BigQuerySink implements Sink<RowData> { | ||
protected BigQueryConnectionOptions options; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
protected BigQueryConnectionOptions options; | |
final protected BigQueryConnectionOptions options; |
@@ -35,4 +35,28 @@ public class BigQueryConfigOptions { | |||
.enumType(DeliveryGuarantee.class) | |||
.defaultValue(DeliveryGuarantee.AT_LEAST_ONCE) | |||
.withDescription("Determines delivery guarantee"); | |||
|
|||
public static final ConfigOption<Long> MAX_OUTSTANDING_ELEMENTS_COUNT = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a suggestion -- the wording "maxOutstandingElementsCount" is from the Google API, but other Flink sinks sometimes use maxInflightRequests Do you think it's worthwhile aligning to other connectors?
I have some doubts ... the semantics are pretty similar but maybe not entirely the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it's from google api since currently it is for google api
we could think about renaming once we switch to async sink Flink connector interfaces
thanks for having a look @RyanSkraba |