Just to double check are the changes in the PR of Thomas even touching the API or would that be another PR? Or are the API changes contained to FLINK-20379?
On Wed, May 5, 2021 at 8:29 PM Arvid Heise <ar...@apache.org> wrote: > After looking a bit more into it, I'm also +1. > > Let's merge it soonish. > > On Wed, May 5, 2021 at 7:20 PM Thomas Weise <t...@apache.org> wrote: > >> I opened the PR to backport the changes: >> https://github.com/apache/flink/pull/15840 >> >> Without these fixes the new KafkaSource in 1.12 is near unusable. The most >> obvious problem I ran into during testing was that checkpoints fail when >> consumption has not started for a split (easily reproduced with a topic >> partition that does not have data). >> >> Thanks, >> Thomas >> >> >> On Tue, Apr 13, 2021 at 11:33 AM Stephan Ewen <se...@apache.org> wrote: >> >> > Hi all! >> > >> > Generally, avoiding API changes in Bug fix versions is the right thing, >> in >> > my opinion. >> > >> > But this case is a bit special, because we are changing something that >> > never worked properly in the first place. >> > So we are not breaking a "running thing" here, but making it usable. >> > >> > So +1 from my side to backport these changes, I think we make more users >> > happy than angry with this. >> > >> > Best, >> > Stephan >> > >> > >> > On Thu, Apr 8, 2021 at 11:35 AM Becket Qin <becket....@gmail.com> >> wrote: >> > >> > > Hi Arvid, >> > > >> > > There are interface changes to the Kafka source, and there is a >> backwards >> > > compatible change in the base source implementation. Therefore >> > technically >> > > speaking, users might be able to run the Kafka source in 1.13 with a >> 1.12 >> > > Flink job. However, it could be tricky because there might be some >> > > dependent jar conflicts between 1.12 and 1.13. So this solution seems >> a >> > > little fragile. >> > > >> > > I'd second Till's question if there is an issue for users that start >> with >> > > > the current Kafka source (+bugfixes) to later upgrade to 1.13 Kafka >> > > source >> > > > with API changes. >> > > >> > > >> > > Just to clarify, the bug fixes themselves include API changes, they >> are >> > not >> > > separable. So we basically have three options here: >> > > >> > > 1. Do not backport fixes in 1.12. So users have to upgrade to 1.13 in >> > order >> > > to use the new Kafka source. >> > > 2. Write some completely different fixes for release 1.12 and ask >> users >> > to >> > > migrate to the new API when they upgrade to 1.13 >> > > 3. Backport the fix with API changes to 1.12. So users don't need to >> > handle >> > > interface change when they upgrade to 1.13+. >> > > >> > > Personally I think option 3 here is better because it does not really >> > > introduce any trouble to the users. The downside is that we do need to >> > > change the API of Kafka source in 1.12. Given that the changed API >> won't >> > be >> > > useful without these bug fixes, changing the API seems to be doing >> more >> > > good than bad here. >> > > >> > > Thanks, >> > > >> > > Jiangjie (Becket) Qin >> > > >> > > >> > > >> > > On Thu, Apr 8, 2021 at 2:39 PM Arvid Heise <ar...@apache.org> wrote: >> > > >> > > > Hi Becket, >> > > > >> > > > did you need to change anything to the source interface itself? >> > Wouldn't >> > > it >> > > > be possible for users to simply use the 1.13 connector with their >> Flink >> > > > 1.12 deployment? >> > > > >> > > > I think the late-upgrade argument can be made for any feature, but I >> > also >> > > > see that the Kafka connector is of high interest. >> > > > >> > > > I'd second Till's question if there is an issue for users that start >> > with >> > > > the current Kafka source (+bugfixes) to later upgrade to 1.13 Kafka >> > > source >> > > > with API changes. >> > > > >> > > > Best, >> > > > >> > > > Arvid >> > > > >> > > > On Thu, Apr 8, 2021 at 2:54 AM Becket Qin <becket....@gmail.com> >> > wrote: >> > > > >> > > > > Thanks for the comment, Till and Thomas. >> > > > > >> > > > > As far as I know there are some users who have just upgraded their >> > > Flink >> > > > > version from 1.8 / 1.9 to Flink 1.12 and might not upgrade the >> > version >> > > > in 6 >> > > > > months or more. There are also some organizations that have the >> > > strategy >> > > > of >> > > > > not running the latest version of a project, but only the second >> > latest >> > > > > version with bug fixes. So those users may still benefit from the >> > > > backport. >> > > > > However, arguably the old Kafka source is there anyways in 1.12, >> so >> > > they >> > > > > should not be blocked on having the new source. >> > > > > >> > > > > I am leaning towards backporting the fixes mainly because this >> way we >> > > > might >> > > > > have more users migrating to the new Source and provide feedback. >> It >> > > will >> > > > > take some time for the users to pick up 1.13, especially for the >> > users >> > > > > running Flink at large scale. So backporting the fixes to 1.12 >> would >> > > help >> > > > > get the new source to be used sooner. >> > > > > >> > > > > Thanks, >> > > > > >> > > > > Jiangjie (Becket) Qin >> > > > > >> > > > > On Thu, Apr 8, 2021 at 12:40 AM Thomas Weise <t...@apache.org> >> wrote: >> > > > > >> > > > > > Hi, >> > > > > > >> > > > > > Thanks for fixing the new KafkaSource issues. >> > > > > > >> > > > > > I'm interested in using these fixes with 1.12 for experimental >> > > > purposes. >> > > > > > >> > > > > > +1 for backporting. 1.12 is the current stable release and users >> > who >> > > > > would >> > > > > > like to try the FLIP-27 sources are likely to use that release. >> > > > > > >> > > > > > Thomas >> > > > > > >> > > > > > On Wed, Apr 7, 2021 at 2:50 AM Till Rohrmann < >> trohrm...@apache.org >> > > >> > > > > wrote: >> > > > > > >> > > > > > > Hi Becket, >> > > > > > > >> > > > > > > If I remember correctly, then we deliberately not documented >> the >> > > > Kafka >> > > > > > > connector in the 1.12 release. Hence, from this point there >> > should >> > > be >> > > > > no >> > > > > > > need to backport any fixes because users are not aware of this >> > > > feature. >> > > > > > > >> > > > > > > On the other hand this also means that we should be able to >> break >> > > > > > anything >> > > > > > > we want to. Consequently, backporting these fixes should be >> > > possible. >> > > > > > > >> > > > > > > The question would probably be whether we want to ship new >> > features >> > > > > with >> > > > > > a >> > > > > > > bug fix release. Do we know of any users who want to use the >> new >> > > > Kafka >> > > > > > > source, are using the 1.12 version and cannot upgrade to 1.13 >> > once >> > > it >> > > > > is >> > > > > > > released? If this is the case, then this could be an argument >> for >> > > > > > shipping >> > > > > > > this feature with a bug fix release. If not, then we could >> save >> > > some >> > > > > work >> > > > > > > by not backporting it. >> > > > > > > >> > > > > > > Cheers, >> > > > > > > Till >> > > > > > > >> > > > > > > On Wed, Apr 7, 2021 at 10:43 AM Becket Qin < >> becket....@gmail.com >> > > >> > > > > wrote: >> > > > > > > >> > > > > > > > Hi folks, >> > > > > > > > >> > > > > > > > I'd like to start a discussion thread about backporting some >> > > > FLIP-27 >> > > > > > > Kafka >> > > > > > > > source connector fixes to release-1.12. These fixes include >> > some >> > > > API >> > > > > > > > changes and thus needs a public discussion. >> > > > > > > > >> > > > > > > > The tickets in question are following: >> > > > > > > > https://issues.apache.org/jira/browse/FLINK-20379 >> > > > > > > > https://issues.apache.org/jira/browse/FLINK-20114 >> > > > > > > > https://issues.apache.org/jira/browse/FLINK-21817 >> > > > > > > > >> > > > > > > > Without these fixes, the FLIP-27 Kafka source in >> release-1.12 >> > is >> > > > not >> > > > > > > really >> > > > > > > > usable, and the API changes only affect the Kafka Source. >> So it >> > > > seems >> > > > > > > > breaking the API in this case is still worthwhile. >> > > > > > > > >> > > > > > > > It would be good to see what others think. >> > > > > > > > >> > > > > > > > Thanks, >> > > > > > > > >> > > > > > > > Jiangjie (Becket) Qin >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> >
