You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/11/01 12:57:00 UTC
[GitHub] [iceberg] openinx opened a new pull request #3434: Flink: Support flink 1.14.0
openinx opened a new pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434
Addressing the issue: https://github.com/apache/iceberg/issues/3433
There are following sub-issues that we need to address:
1. The apache flink has removed the blink module and made it as the default planner, so we don't need to add the `org.apache.flink:flink-table-planner-blink_2.12` explicitly in the dependencies.
2. There are following build issues when build the `./gradlew -DflinkVersions=1.14 :iceberg-flink:iceberg-flink-1.14:build`:
```
> Task :iceberg-flink:iceberg-flink-1.14:compileJava
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/source/StreamingReaderOperator.java:32: error: cannot find symbol
import org.apache.flink.streaming.api.operators.MailboxExecutor;
^
symbol: class MailboxExecutor
location: package org.apache.flink.streaming.api.operators
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/source/StreamingReaderOperator.java:65: error: cannot find symbol
private final MailboxExecutor executor;
^
symbol: class MailboxExecutor
location: class StreamingReaderOperator
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/source/StreamingReaderOperator.java:80: error: cannot find symbol
MailboxExecutor mailboxExecutor) {
^
symbol: class MailboxExecutor
location: class StreamingReaderOperator
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/source/StreamingReaderOperator.java:208: error: cannot find symbol
private transient MailboxExecutor mailboxExecutor;
^
symbol: class MailboxExecutor
location: class OperatorFactory
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/source/StreamingReaderOperator.java:215: error: cannot find symbol
public void setMailboxExecutor(MailboxExecutor mailboxExecutor) {
^
symbol: class MailboxExecutor
location: class OperatorFactory
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/source/StreamingReaderOperator.java:113: error: cannot find symbol
getContainingTask().getStreamStatusMaintainer(),
^
symbol: method getStreamStatusMaintainer()
location: class StreamTask<CAP#1,CAP#2>
where CAP#1,CAP#2 are fresh type-variables:
CAP#1 extends Object from capture of ?
CAP#2 extends StreamOperator<CAP#1> from capture of ?
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/source/StreamingReaderOperator.java:171: error: method does not override or implement a method from a supertype
@Override
^
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/source/StreamingReaderOperator.java:173: error: cannot find symbol
super.dispose();
^
symbol: method dispose()
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/sink/IcebergFilesCommitter.java:347: error: method does not override or implement a method from a supertype
@Override
^
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/sink/IcebergStreamWriter.java:75: error: method does not override or implement a method from a supertype
@Override
^
/Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/sink/IcebergStreamWriter.java:77: error: cannot find symbol
super.dispose();
^
symbol: method dispose()
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: /Users/openinx/software/apache-iceberg/flink/src/main/java/org/apache/iceberg/flink/data/RowDataUtil.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
11 errors
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961465752
I am with copying all code to version specific module so that we don't have to decide which classes kept in common module vs version modules. This would be consistent with Spark version handling too.
I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only. After it is done, we can copy code to 1.14 module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961587324
> I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only.
The FLIP-27 API changes between 1.12 and 1.13 is caused by this [FLINK-22133](https://issues.apache.org/jira/browse/FLINK-22133), the flink changed the basic snapshotState API, that means when @stevenzwu develop the flip-27 source reader , he will encounter the same problem as I have encountered in this issue. If we plan to keep the flink common codes in `iceberg-flink`, then we need to pick up all the upper layer classes which is built on top of `snapshotState` API to the `:iceberg-flink:iceberg-flink-1.12` and `:iceberg-flink:iceberg-flink-1.13`. I have tried to do the pick up when supporting flink 1.14, this is a very tedious and error-prone work. so finally, I gave up the picking up work and turned to clone a same code base for flink 1.14.
As the increase of newly introduced features for flink+iceberg integration work, I think we will encounter more and more under layer API changes , that means we will need to copy more and more classes from the common flink module to the specific versioned module. Rather than splitting the work among different Flink contributors (It is very easy to miss classes during development and review, resulting in missing tests and features), I will prefer to clone the separate code base for each flink specified version.
In last week, I prefer to share the common code for flink 1.12 & flink1.13. But I changed my mind when I port the integration work for flink1.14 and release that @stevenzwu 's flip-27 is encountering the similar issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] SteNicholas removed a comment on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
SteNicholas removed a comment on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-971149619
@openinx , what about the integration with the new `Sink` interface? The FLIP-27 is implemented by @stevenzwu , and the corresponding FLIP-143 support is necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961522740
I don't have a preference about this for Flink. I think that copying will work well for Spark, but I think it's up to the people working on Flink whether to use the same strategy.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961587324
> I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only.
The FLIP-27 API changes between 1.12 and 1.13 is caused by this [FLINK-22133](https://issues.apache.org/jira/browse/FLINK-22133), the flink changed the basic snapshotState API, that means when @stevenzwu develop the flip-27 source reader , he will encounter the same problem as I have encountered in this issue. If we plan to keep the flink common codes in `iceberg-flink`, then we need to pick up all the upper layer classes which is built on top of `snapshotState` API to the `:iceberg-flink:iceberg-flink-1.12` and `:iceberg-flink:iceberg-flink-1.13`. I have tried to do the pick up when supporting flink 1.14, this is a very tedious and error-prone work. so finally, I gave up the picking up work and turned to clone a same code base for flink 1.14.
As the increase of newly introduced features for flink+iceberg integration work, I think we will encounter more and more under layer API changes , that means we will need to copy more and more classes from the common flink module to the specific versioned module. Rather than splitting the work among different Flink contributors (It is very easy to miss classes during development and review, resulting in missing tests and features), I will prefer to clone the separate code base for each flink specified version.
In last week, I prefer to share the common code for flink 1.12 & flink1.13. But I changed my mind when I port the integration work for flink1.14 and release that @stevenzwu 's flip-27 is encountering the similar issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-987510546
I think this PR will need an update now because I think there are some newly introduced changes which were added into flin 1.13.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-956354718
@openinx, it looks like the tests are passing for this in the latest commit. Did you solve the issues from the description? Just let us know when you're ready for a review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-956354718
@openinx, it looks like the tests are passing for this in the latest commit. Did you solve the issues from the description? Just let us know when you're ready for a review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961587324
> I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only.
The FLIP-27 API changes between 1.12 and 1.13 is caused by this [FLINK-22133](https://issues.apache.org/jira/browse/FLINK-22133), the flink changed the basic snapshotState API, that means when @stevenzwu develop the flip-27 source reader , he will encounter the same problem as I have encountered in this issue. If we plan to keep the flink common codes in `iceberg-flink`, then we need to pick up all the upper layer classes which is built on top of `snapshotState` API to the `:iceberg-flink:iceberg-flink-1.12` and `:iceberg-flink:iceberg-flink-1.13`. I have tried to do the pick up when supporting flink 1.14, this is a very tedious and error-prone work. so finally, I gave up the picking up work and turned to clone a same code base for flink 1.14.
As the increase of newly introduced features for flink+iceberg integration work, I think we will encounter more and more under layer API changes , that means we will need to copy more and more classes from the common flink module to the specific versioned module. Rather than splitting the work among different Flink contributors (It is very easy to miss classes during development and review, resulting in missing tests and features), I will prefer to clone the separate code base for each flink specified version.
In last week, I prefer to share the common code for flink 1.12 & flink1.13. But I changed my mind when I port the integration work for flink1.14 and release that @stevenzwu 's flip-27 is encountering the similar issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] SteNicholas commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-971149619
@openinx , what about the integration with the new `Sink` interface? The FLIP-27 is implemented by @stevenzwu , and the corresponding FLIP-143 support is necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-957030598
> Did you solve the issues from the description?
I'm still working on this to resolve the incompatibility issues between flink 1.13 and flink 1.14. The build seems does not run all the unit tests on top of flink 1.14, I left another comment in this [issue](https://github.com/apache/iceberg/pull/2305/files#r740086825). Will publish another PR to address the issues about unit tests cross flink versions.
If I think this PR is ready, I will ping you, @stevenzwu and @kbendick for reviewing. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-964090925
Ping @rdblue , any other concern ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961465752
I am with copying all code to version specific module so that we don't have to decide which classes kept in common module vs version modules. This would be consistent with Spark version handling too.
I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only. After it is done, we can copy code to 1.14 module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961522740
I don't have a preference about this for Flink. I think that copying will work well for Spark, but I think it's up to the people working on Flink whether to use the same strategy.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx edited a comment on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx edited a comment on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961587324
> I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only.
The FLIP-27 API changes between 1.12 and 1.13 is caused by this [FLINK-22133](https://issues.apache.org/jira/browse/FLINK-22133), the flink changed the basic snapshotState API, that means when @stevenzwu develop the flip-27 source reader , he will encounter the same problem as I have encountered in this issue. If we plan to keep the flink common codes in `iceberg-flink`, then we need to pick up all the upper layer classes which is built on top of `snapshotState` API to the `:iceberg-flink:iceberg-flink-1.12` and `:iceberg-flink:iceberg-flink-1.13`. I have tried to do the pick up when supporting flink 1.14, this is a very tedious and error-prone work. so finally, I gave up the picking up work and turned to clone a same code base for flink 1.14.
As the increase of newly introduced features for flink+iceberg integration work, I think we will encounter more and more under layer API changes , that means we will need to copy more and more classes from the common flink module to the specific versioned module. Rather than splitting the work among different Flink contributors (It is very easy to miss classes during development and review, resulting in missing tests and features), I will prefer to clone the separate code base for each flink specified version.
In last week, I prefer to share the common code for flink 1.12 & flink1.13. But I changed my mind when I port the integration work for flink1.14 and realize that @stevenzwu 's flip-27 is encountering the similar issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu edited a comment on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961465752
I am fine with copying all code to version specific module so that we don't have to decide which classes kept in common module vs version modules. This would be consistent with Spark version handling too.
I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only. After it is done, we can copy code to 1.14 module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961465752
I am with copying all code to version specific module so that we don't have to decide which classes kept in common module vs version modules. This would be consistent with Spark version handling too.
I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only. After it is done, we can copy code to 1.14 module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] SteNicholas commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-971149890
@openinx , what about the integration with the new `Sink` interface? The FLIP-27 is implemented by @stevenzwu , and the corresponding FLIP-143 support is necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-971374709
> @openinx , what about the integration with the new `Sink` interface? The FLIP-27 is implemented by @stevenzwu , and the corresponding FLIP-143 support is necessary.
I think the flip-27 sink integration work will be merged in flink 1.13 module firstly, and then port other related flink versions. That is a bigger feature and we need to make sure it works stable for a specific flink version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx edited a comment on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx edited a comment on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961587324
> I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only.
The FLIP-27 API changes between 1.12 and 1.13 is caused by this [FLINK-22133](https://issues.apache.org/jira/browse/FLINK-22133), the flink changed the basic snapshotState API, that means when @stevenzwu develop the flip-27 source reader , he will encounter the same problem as I have encountered in this issue. If we plan to keep the flink common codes in `iceberg-flink`, then we need to pick up all the upper layer classes which is built on top of `snapshotState` API to the `:iceberg-flink:iceberg-flink-1.12` and `:iceberg-flink:iceberg-flink-1.13`. I have tried to do the pick up when supporting flink 1.14, this is a very tedious and error-prone work. so finally, I gave up the picking up work and turned to clone a same code base for flink 1.14.
As the increase of newly introduced features for flink+iceberg integration work, I think we will encounter more and more under layer API changes , that means we will need to copy more and more classes from the common flink module to the specific versioned module. Rather than splitting the work among different Flink contributors (It is very easy to miss classes during development and review, resulting in missing tests and features), I will prefer to clone the separate code base for each flink specified version.
In last week, I prefer to share the common code for flink 1.12 & flink1.13. But I changed my mind when I port the integration work for flink1.14 and realize that @stevenzwu 's flip-27 is encountering the similar issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
stevenzwu commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-971882674
@openinx I think you meant flip-17 source integration will be merged to flink 1.13 module first.
@SteNicholas was asking about the new flip-143 unified sink interface, which I don't think anyone has picked that one up yet. We probably should create a separate github issue to track that work and discussions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-957030598
> Did you solve the issues from the description?
I'm still working on this to resolve the incompatibility issues between flink 1.13 and flink 1.14. The build seems does not run all the unit tests on top of flink 1.14, I left another comment in this [issue](https://github.com/apache/iceberg/pull/2305/files#r740086825). Will publish another PR to address the issues about unit tests cross flink versions.
If I think this PR is ready, I will ping you, @stevenzwu and @kbendick for reviewing. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-957030598
> Did you solve the issues from the description?
I'm still working on this to resolve the incompatibility issues between flink 1.13 and flink 1.14. The build seems does not run all the unit tests on top of flink 1.14, I left another comment in this [issue](https://github.com/apache/iceberg/pull/2305/files#r740086825). Will publish another PR to address the issues about unit tests cross flink versions.
If I think this PR is ready, I will ping you, @stevenzwu and @kbendick for reviewing. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961522740
I don't have a preference about this for Flink. I think that copying will work well for Spark, but I think it's up to the people working on Flink whether to use the same strategy.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu edited a comment on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961465752
I am fine with copying all code to version specific module so that we don't have to decide which classes kept in common module vs version modules. This would be consistent with Spark version handling too.
I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only. After it is done, we can copy code to 1.14 module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-986938823
I'll take a look today. Thanks, @openinx! And thanks to everyoen for reviewing!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-988077371
Thanks, @openinx!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] stevenzwu edited a comment on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
stevenzwu edited a comment on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961465752
I am fine with copying all code to version specific module so that we don't have to decide which classes kept in common module vs version modules. This would be consistent with Spark version handling too.
I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only. After it is done, we can copy code to 1.14 module.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-962883471
Since we've got this #3476 merged, let's update this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx edited a comment on pull request #3434: Flink: Support flink 1.14.0
Posted by GitBox <gi...@apache.org>.
openinx edited a comment on pull request #3434:
URL: https://github.com/apache/iceberg/pull/3434#issuecomment-961587324
> I would ask we also duplicate the code in 1.12 and 1.13 too. FLIP-27 Flink source will have different code in 1.13. We may even skip 1.12 for FLIP-27 source. Then I can start with the FLIP-27 source in 1.13 only.
The FLIP-27 API changes between 1.12 and 1.13 is caused by this [FLINK-22133](https://issues.apache.org/jira/browse/FLINK-22133), the flink changed the basic snapshotState API, that means when @stevenzwu develop the flip-27 source reader , he will encounter the same problem as I have encountered in this issue. If we plan to keep the flink common codes in `iceberg-flink`, then we need to pick up all the upper layer classes which is built on top of `snapshotState` API to the `:iceberg-flink:iceberg-flink-1.12` and `:iceberg-flink:iceberg-flink-1.13`. I have tried to do the pick up when supporting flink 1.14, this is a very tedious and error-prone work. so finally, I gave up the picking up work and turned to clone a same code base for flink 1.14.
As the increase of newly introduced features for flink+iceberg integration work, I think we will encounter more and more under layer API changes , that means we will need to copy more and more classes from the common flink module to the specific versioned module. Rather than splitting the work among different Flink contributors (It is very easy to miss classes during development and review, resulting in missing tests and features), I will prefer to clone the separate code base for each flink specified version.
In last week, I prefer to share the common code for flink 1.12 & flink1.13. But I changed my mind when I port the integration work for flink1.14 and realize that @stevenzwu 's flip-27 is encountering the similar issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org