Flink : upgrade to flink 1.13 #2629

zhangjun0x01 · 2021-05-24T10:05:49Z

upgrade to flink 1.13
#2558

GH-Gloway · 2021-05-27T01:47:02Z

flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactoryOptions.java

+          .stringType()
+          .defaultValue("true")
+          .withDescription("Whether to cache the catalog in FlinkCatalog.");
+}


missing option default-database

This is some options for using sql to create catalog, I think we don’t need to add default-database , http://iceberg.apache.org/flink/#creating-catalogs-and-using-catalogs

nastra · 2021-06-08T07:16:20Z

There are a bunch of docs that still mention an older version of Flink. Should we update those as well as part of this PR?

zhangjun0x01 · 2021-06-08T08:52:54Z

There are a bunch of docs that still mention an older version of Flink. Should we update those as well as part of this PR?

At present, the community has not reached an agreement whether to upgrade to flink 1.13, if you need to upgrade to flink 1.13. You can apply the PR to your branch first

gaborgsomogyi · 2021-06-08T14:31:42Z

FYI https://flink.apache.org/news/2021/05/28/release-1.13.1.html

gaborgsomogyi · 2021-06-09T08:58:53Z

I've tested the changes and works fine with 1.13.0.

mbalassi · 2021-06-22T12:51:40Z

Are these changes backwards compatible with Flink 1.12 too or does it strictly require 1.13?

stevenzwu · 2021-08-30T01:47:48Z

flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactoryOptions.java

+              "Version number to describe the property version. This property can be used for backwards " +
+                  "compatibility in case the property format changes. The current property version is `1`. (Optional)");
+
+  public static final ConfigOption<String> URI =


can we reuse some constants from CatalogProperties in iceberg-core module? like

ConfigOptions.key(CatalogProperties.URI)

stevenzwu

LGTM. I am also +1 for upgrading to 1.13

kbendick

Thanks for working on this. Some people are interested in being able to use Flink + Iceberg in environments where Hadoop is not easily configured, which we'd thought that the access to the class loader provided by the new CatalogFactory.Context might make easier.

I don't want to block this PR, so I've left some comments. My only outstanding concern is the change from using * in supportedOptons to the defined list in optionalOptions.

Left a few comments with some configs that I know of.

kbendick · 2021-08-30T19:03:20Z

flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java

+        FactoryUtil.createCatalogFactoryHelper(this, context);
+    helper.validate();
+
+    return createCatalog(context.getName(), context.getOptions(), clusterHadoopConf());


There is an effort to decouple the need for Hadoop from the FlinkCatalogFactory for environments where Hadoop is not easily configurable and for catalog implementations that don't actually need it (basically anything that uses S3FileIO - though we'll need to update GlueCatalog and eventually DynamoDbCatalog as well).

Even after updating GlueCatalog to remove the Hadoop Configurable interface, this call to clusterHadoopConf() still makes it so that Hadoop is needed on the class path.

I'm not proposing that we change that in this PR (as this PR has been open for a while, and it's a separate concern), but I wanted to draw attention to the issue as I'm a bit less informed on the Flink side compared to many of you: #3044

kbendick · 2021-08-30T19:23:00Z

flink/src/main/java/org/apache/iceberg/flink/FlinkCatalogFactory.java

+  public Set<ConfigOption<?>> optionalOptions() {
+    final Set<ConfigOption<?>> options = new HashSet<>();
+    options.add(FlinkCatalogFactoryOptions.PROPERTY_VERSION);
+    options.add(FlinkCatalogFactoryOptions.URI);
+    options.add(FlinkCatalogFactoryOptions.WAREHOUSE);
+    options.add(FlinkCatalogFactoryOptions.CLIENTS);
+    options.add(FlinkCatalogFactoryOptions.BASE_NAMESPACE);
+    options.add(FlinkCatalogFactoryOptions.HIVE_CONF_DIF);
+    options.add(FlinkCatalogFactoryOptions.CACHE_ENABLED);
+    return options;
+  }


Do we need to add any of the known additional catalog properties that are used with some of the AWS catalogs?

Also, for catalogs that offer additional options (including custom catalogs), how will users set those? Presently, supportedOptions has a *. Is that still possible? The usage of * would be the easiest path forward, but that might not be possible.

If not, the ones I can think of are presently coming from the AWS catalogs (though we should also look into the JDBC catalog as well).

Additional Catalog Properties To Consider:
General Catalog Properties

catalog-impl

io-impl

GlueCatalog Specific Options

lock-impl

lock.table

DynamoDbCatalog Specific Options

dynamodb.table-name

AWS Client Configuration

client.factory

There are further ones which I'll try to link to in a bit (such as S3FileIO options for AWS authentication and encryption parameters), but those are the first ones I encountered in the docs.

Also, will users be able to specify their own catalog implementation (with its own options) still?

kbendick · 2021-08-31T19:28:53Z

@tiborkiss mentioned in the apache-iceberg Slack that this PR would also be needed to fully support Flink SQL (and not just streaming apps): #2666

openinx · 2021-09-02T06:03:15Z

@zhangjun0x01 , I think it's time to upgrade the version to flink 1.13 now. Would you like to fix the conflicts ? I'd like to check whether it's OK to get this merge..

Flyangz · 2021-09-08T08:21:59Z

@zhangjun0x01 , are you still working on it? If not or you do not have time to fix it, I would like to pick it up.

openinx · 2021-09-26T08:13:36Z

Closed via #3116

upgrade to flink 1.13

a021b12

github-actions bot added build flink labels May 24, 2021

base-namespace

bd36845

GH-Gloway reviewed May 27, 2021

View reviewed changes

coolderli mentioned this pull request Aug 12, 2021

赶紧支持flink1.13啊大佬们 #2959

Closed

stevenzwu mentioned this pull request Aug 30, 2021

Update FlinkCatalogFactory to implement the new Factory interface added to Flink's CatalogFactory [Partial Duplicate of Issue #2558] #3050

Closed

stevenzwu reviewed Aug 30, 2021

View reviewed changes

stevenzwu approved these changes Aug 30, 2021

View reviewed changes

kbendick reviewed Aug 30, 2021

View reviewed changes

kbendick mentioned this pull request Aug 30, 2021

Unable to use GlueCatalog in flink environments without hadoop #3044

Closed

openinx mentioned this pull request Sep 8, 2021

Fix ClassCastException when flink read map and array data from parquet format data file. #3081

Closed

Flyangz mentioned this pull request Sep 14, 2021

Flink: Upgrade to flink 1.13.2 #3116

Merged

openinx closed this Sep 26, 2021

Flink : upgrade to flink 1.13 #2629

Flink : upgrade to flink 1.13 #2629

Uh oh!

Conversation

zhangjun0x01 commented May 24, 2021

Uh oh!

GH-Gloway May 27, 2021

Choose a reason for hiding this comment

Uh oh!

zhangjun0x01 May 27, 2021

Choose a reason for hiding this comment

Uh oh!

nastra commented Jun 8, 2021

Uh oh!

zhangjun0x01 commented Jun 8, 2021

Uh oh!

gaborgsomogyi commented Jun 8, 2021

Uh oh!

gaborgsomogyi commented Jun 9, 2021

Uh oh!

mbalassi commented Jun 22, 2021

Uh oh!

stevenzwu Aug 30, 2021

Choose a reason for hiding this comment

Uh oh!

stevenzwu left a comment

Choose a reason for hiding this comment

Uh oh!

kbendick left a comment

Choose a reason for hiding this comment

Uh oh!

kbendick Aug 30, 2021

Choose a reason for hiding this comment

Uh oh!

kbendick Aug 30, 2021

Choose a reason for hiding this comment

Uh oh!

kbendick Aug 30, 2021

Choose a reason for hiding this comment

Uh oh!

kbendick commented Aug 31, 2021

Uh oh!

openinx commented Sep 2, 2021

Uh oh!

Flyangz commented Sep 8, 2021

Uh oh!

openinx commented Sep 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants