Once we deprecate FlinkSink, we should also upgrade IcebergSink from `Experimental` to `PublicEvolving`
On Wed, Mar 12, 2025 at 10:01 AM Steven Wu <stevenz...@gmail.com> wrote: > Max, thanks for confirming the Flink 2.0 behavior. With the latest > information, here are my suggestions > > 1) I am fine with deleting the old `FlinkSource` in the upcoming flink 2.0 > module (but keeping them in flink 1.x modules). As discussed in a previous > thread, the new FLIP-27 source has been baked by users for 2 years. We > already switched SQL to the new FLIP-27 source in the Iceberg 1.7 release. > So far so good. > > 2) for FlinkSink, I would prefer to keep it around with flink 2.0 module. > Here are my reasons. > - new sink is still relatively young. Give it more time to bake and mature. > - Flink sink (for streaming ingestion) is probably a lot more widely used > than Flink source (for batch or streaming read). Hence, it is valuable to > be more conservative in the deprecation plan here. > - we can stop adding new features to the old FlinkSink and only contribute > to the new IcebergSink. that should reduce the maintenance burden. > - we can mark the FinkSink as deprecated for all flink versions once the > parity is achieved. > > > On Wed, Mar 12, 2025 at 7:09 AM Maximilian Michels <m...@apache.org> wrote: > >> Hi JB, >> >> V3 is another topic, but it would be good to start looking into that. >> >> Hey Steven, >> >> Flink 2.0 actually did not remove the old interfaces, which was not >> what I expected when I started this conversation. Turns out, old >> interfaces have been moved to ".legacy" packages. I'm in the process >> of verifying they will continue to work as expected. >> >> It looks like we will still have the option to keep around the old >> source / sink implementations. However, I think it still makes sense >> to remove FlinkSource and FlinkSink in the Flink 2.0 code path. This >> will simplify the code base and avoid confusion for users in the long >> run. I don't see real value in bringing over legacy sources / sinks to >> a new Flink major release. >> >> -Max >> >> On Tue, Mar 11, 2025 at 10:46 PM Steven Wu <stevenz...@gmail.com> wrote: >> > >> > I assume Flink 2.0 will remove the old source and sink interfaces. >> > >> > With the above assumption (please correct me if I am wrong), here is >> what I would suggest for the next Iceberg release (1.9 or whichever version >> that lands the Flink 2.0 support). >> > - Flink 2.0 module - remove the old source and sink impls. we have no >> choice here. >> > - Flink 1.x modules - update the deprecation note for old `FlinkSource` >> that will be removed in Flink 2.0 support. add a similar deprecation node >> for old `FlinkSink`. >> > >> > We shouldn't remove the old `FlinkSource` and `FlinkSink` for the Flink >> 1.x modules. Allow users to continue to use them with Flink 1.x. Note that >> Iceberg supports 3 active Flink versions (currently 1.18, 1.19, 1.20). So >> old versions will be naturally retired over time. As Flink progresses with >> 2.0, 2.1, 2.2 (etc.), we may want to keep the support for the last Flink >> 1.x version a little longer based on the community feedback. >> > >> > Thanks, >> > Steven >> > >> > On Tue, Mar 11, 2025 at 8:48 AM Jean-Baptiste Onofré <j...@nanthrax.net> >> wrote: >> >> >> >> Hi Max >> >> >> >> Yes, it makes sense. Maybe I can help on that as I'm working on Spec >> >> V3 support in Flink (I did the default value, and I'm checking the new >> >> types now). >> >> >> >> Please let me know :) >> >> >> >> Thanks, >> >> Regards >> >> JB >> >> >> >> On Tue, Mar 11, 2025 at 11:23 AM Maximilian Michels <m...@apache.org> >> wrote: >> >> > >> >> > Hi JB, >> >> > >> >> > Yes, Flink 2.0 support ideally should land in Iceberg 1.9. >> >> > >> >> > Hi Rodrigo, >> >> > >> >> > +1 on closing the gap between the two sink implementations: FlinkSink >> >> > and IcebergSink. Thanks for helping to close that gap! >> >> > >> >> > Hi Steven, >> >> > >> >> > Good point on the "Remove by Iceberg 2.0" claim stated in >> FlinkSource. >> >> > We can keep it until Iceberg 2.0 in the Flink 1.X paths, but I would >> >> > propose to remove FlinkSource earlier than Iceberg 2.0 for the Flink >> >> > 2.X path. The reason is that it is not used by default for many >> >> > Iceberg versions. I think it is unlikely that users jumping on Flink >> >> > 2.0 will want to use it. >> >> > >> >> > As for removing FlinkSink, you're right that IcebergSink is >> relatively >> >> > new as a replacement. If we can keep it in the process of porting the >> >> > code, we may keep it around longer, but I suggest deprecating it and >> >> > scheduling it for removal in the Iceberg release following 1.9. >> >> > >> >> > To summarize, I'm proposing the following: >> >> > >> >> > Iceberg 1.9 >> >> > - Remove FlinkSource in Flink 2.0 code path >> >> > - Deprecate FlinkSink for all supported Flink versions >> >> > >> >> > Iceberg > 1.9 >> >> > - Remove FlinkSource for all supported Flink versions >> >> > - Remove FlinkSink for all supported Flink versions >> >> > >> >> > Does that sound right? >> >> > >> >> > Cheers, >> >> > Max >> >> > >> >> > On Thu, Mar 6, 2025 at 9:10 PM Steven Wu <stevenz...@gmail.com> >> wrote: >> >> > > >> >> > > > if Flink 2.0 release is going to release the old source and sink >> interfaces, t >> >> > > >> >> > > typo above: "release the old" -> "delete the old" >> >> > > >> >> > > On Thu, Mar 6, 2025 at 12:08 PM Steven Wu <stevenz...@gmail.com> >> wrote: >> >> > >> >> >> > >> There was a previous thread for the Flink source. The Javadoc >> deprecation note for the old `FlinkSource` currently says it will be >> removed in Iceberg 2.0 release >> >> > >> https://lists.apache.org/thread/27kcvo3p86pysk9wrggq4vphzo03sv3l >> >> > >> >> >> > >> Now regarding deprecating and removing the old `FlinkSink` in >> favor of the new `IcebergSink`, the new `IcebergSink` was added 6 months >> ago and released in Iceberg 1.7 (current release is 1.8). Not sure how many >> users have got a chance to use it. Ideally, I would like to have the new >> `IcebergSink` bake longer with more users running it in the production >> environments. There is also the parity problems that Rod mentioned. >> >> > >> >> >> > >> However, if Flink 2.0 release is going to release the old source >> and sink interfaces, then we will have no choice and remove the old source >> and sink implementations earlier than we originally planned/preferred. >> >> > >> >> >> > >> >> >> > >> On Thu, Mar 6, 2025 at 8:40 AM Rodrigo Meneses < >> rmene...@gmail.com> wrote: >> >> > >>> >> >> > >>> Hi Max, +1 on that too. IcebergSink has been hanging around for >> a while now. We want to make sure we have the same features, though: >> >> > >>> >> >> > >>> I’ve got https://github.com/apache/iceberg/pull/12071 that adds >> the Range Distribution Mode to IcebergSink. It still needs a few recent bug >> fixes and features backported, but it should be ready soon. >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> On Thu, Mar 6, 2025 at 8:18 AM Jean-Baptiste Onofré < >> j...@nanthrax.net> wrote: >> >> > >>>> >> >> > >>>> Hi Max, >> >> > >>>> >> >> > >>>> I guess you are proposing to remove FlinkSink and the >> corresponding >> >> > >>>> FlinkSource as well. >> >> > >>>> >> >> > >>>> It makes to me as, I saw in the code, that both FlinkSink and >> >> > >>>> FlinkSource are deprecated for a while. >> >> > >>>> >> >> > >>>> So, +1 to remove it. >> >> > >>>> >> >> > >>>> Are you planning this for 1.9.0 ? >> >> > >>>> >> >> > >>>> Regards >> >> > >>>> JB >> >> > >>>> >> >> > >>>> On Thu, Mar 6, 2025 at 4:21 PM Maximilian Michels < >> m...@apache.org> wrote: >> >> > >>>> > >> >> > >>>> > Hi, >> >> > >>>> > >> >> > >>>> > Today there are two Flink write connectors in Iceberg: >> >> > >>>> > >> >> > >>>> > 1. FlinkSink (original sink, based on Flink legacy interfaces) >> >> > >>>> > 2. IcebergSink (newer version, based on modern Flink API) >> >> > >>>> > >> >> > >>>> > In terms of features, (1) is a subset of (2). >> >> > >>>> > >> >> > >>>> > I'm in the process of adding support for Flink 2.0. The >> interfaces >> >> > >>>> > used for (1) have been deprecated for several Flink versions >> and are >> >> > >>>> > removed / discouraged in Flink 2.0. >> >> > >>>> > >> >> > >>>> > Therefore, I would like to propose to remove FlinkSink for >> the Flink >> >> > >>>> > 2.0 Iceberg module. We have already deprecated FlinkSink for >> a while. >> >> > >>>> > >> >> > >>>> > Any objections? >> >> > >>>> > >> >> > >>>> > -Max >> >
