Repository navigation
Flink: Add TableCreator interface to set table properties/location on DynamicIcebergSink table creation - #14578
Conversation
06425db to
1ddd45d
Compare
|
I’ve added a few comments on the PR, but the bigger question is deciding what functionality we actually want.
We need to draw a clear line: what should the Dynamic Sink support, and what should be handled by external operators outside the Dynamic Iceberg Sink? To make this decision, I’d like input from actual users: @jordepic, @mxm, @Guosmilesmile, or anyone else interested. Maybe someone could even start a discussion on the dev list to reach a wider audience. If some changes are handled by external operators, we may need a mechanism for those operators to notify the Dynamic Iceberg Sink to refresh its caches. One option could be using something like the Flink Orchestrator restart nonce (https://nightlies.apache.org/flink/flink-kubernetes-operator-docs-main/docs/custom-resource/job-management/#application-restarts-without-spec-change). If the cache detects a new nonce (greater than the current one), it could invalidate the current table cache and reload the data. |
|
I think our requirements, for now, are twofold:
This change is just addressing number 1! I'm happy to expand it to do number 2 as well, though I'm less sure of the business logic that everybody needs there, hence why I excluded it for now. |
1ddd45d to
3805827
Compare
|
Let's start with a narrow scope for this feature. We could always extend it to DynamicRecord, but I'm not convinced the feature should live in DynamicRecord because table properties aren't connected directly to the table data (that's why I kept it outside in #13883). The majority of the use cases will only require setting table properties on table creation. Concerning (2), we've had users request setting the table location as well, so I would suggest to add this feature alongside the table properties on table creation. |
|
Do we need different properties for different tables, or we can use/set the same properties for every table? |
|
I think users will need the table identifier. |
3805827 to
a1aca93
Compare
|
Please rename the PR to match the new feature added |
a1aca93 to
59e291a
Compare
Done. |
The DynamicIcebergSink does not currently allow providing a set of reasonable default table properties at creation time. This change will apply these properties when a table is created (not updated). This helps for specifying things like iceberg version, merge-on-read vs. copy-on-write, table location, and others.
59e291a to
274d118
Compare
pvary
left a comment
There was a problem hiding this comment.
LGTM.
@mxm, @Guosmilesmile: Please review!
|
Merged to main. @jordepic: Please port the changes to the other Flink versions. Please tell us on the PR if the backport was clean, and if not, please highlight the manual changes required. Thanks, |
DynamicIcebergSink table creation (backport) This is a *clean* backport of apache#14578 to Flink 2.0 The DynamicIcebergSink does not currently allow providing a set of reasonable default table properties at creation time. This change will apply these properties when a table is created (not updated). This helps for specifying things like iceberg version, merge-on-read vs. copy-on-write, table location, and others.
DynamicIcebergSink table creation (backport) This is a *clean* backport of apache#14578 to Flink 2.0 The DynamicIcebergSink does not currently allow providing a set of reasonable default table properties at creation time. This change will apply these properties when a table is created (not updated). This helps for specifying things like iceberg version, merge-on-read vs. copy-on-write, table location, and others.
DynamicIcebergSink table creation (backport to 2.0 and 1.20) This is a *clean* backport of apache#14578 to Flink 2.0 There are 2 unclean lines in TestDynamicTableUpdateOperator where we add the TableCreator parameter. The DynamicIcebergSink does not currently allow providing a set of reasonable default table properties at creation time. This change will apply these properties when a table is created (not updated). This helps for specifying things like iceberg version, merge-on-read vs. copy-on-write, table location, and others.
DynamicIcebergSink table creation (backport to 2.0 and 1.20) This is a *clean* backport of apache#14578 to Flink 2.0 There are 2 unclean lines in TestDynamicTableUpdateOperator where we add the TableCreator parameter. The DynamicIcebergSink does not currently allow providing a set of reasonable default table properties at creation time. This change will apply these properties when a table is created (not updated). This helps for specifying things like iceberg version, merge-on-read vs. copy-on-write, table location, and others.
The DynamicIcebergSink does not currently allow providing a set of reasonable default table properties at table creation. This change will apply these properties when a table is created (not updated). This helps for specifying things like iceberg version, merge-on-read vs. copy-on-write, table location, and others.