[Discuss] Breaking API Change in 1.10.1

[Seth Wiesman-4]

Hi Everyone,

I realize I'm about 7 hours late but I just realized there is a breaking
API change in 1.10.1. FLINK-16684 changes the API of the streaming file
sink in a binary incompatible way. Since the release has been approved I'm
not sure the correct course of action but I wanted to bring this to the
communities attention.

Seth

https://issues.apache.org/jira/browse/FLINK-16684

[Thomas Weise]

We also noticed that and had to make an adjustment downstream.

It would be good to mention this in the release notes (if that's not
already the case).

Thomas


On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]> wrote:

> Hi Everyone,
>
> I realize I'm about 7 hours late but I just realized there is a breaking
> API change in 1.10.1. FLINK-16684 changes the API of the streaming file
> sink in a binary incompatible way. Since the release has been approved I'm
> not sure the correct course of action but I wanted to bring this to the
> communities attention.
>
> Seth
>
> https://issues.apache.org/jira/browse/FLINK-16684
>

[Till Rohrmann]

Thanks for reporting this issue Seth. This is indeed a big problem.

I looked into the problem and it seems we have the following situation:

# 1.9.x --> 1.10.0

There is an API breaking change between 1.9.x and 1.10 due FLINK-13864
because it introduced another generic parameter. I expected only few people
will be affected by this because one would have to store the builder in a
variable.

1.9.x is binary compatible with 1.10.0.

# 1.10.0 -> 1.10.1

There is no API breaking change between 1.10.0 and 1.10.1.

I changed the return type of the StreamingFileSink.forRowFormat and
.forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder in
FLINK-16684. This causes the java.lang.NoSuchMethodError and the binary
incompatibility between 1.10.0 and 1.10.1. This is should not happen for
bug fix releases.

W/o FLINK-16684, the StreamingFileSink builders cannot be used with Flink's
Scala API.

# Options

I think we should keep the fix for 1.11.0 and add a release note that we
are violating binary compatibility between 1.10 and 1.11.

Now the question is what to do with the 1.10.x branch:

a) We can revert the change to re-establish binary compatibility between
1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This would also
imply that we cannot use the StreamingFileSink builders with the Scala API.

b) Keep the change and add a release note that our users need to recompile
their jobs. This would allow Flink 1.10.x with x >= 1 users to use
StreamingFileSink builders with the Scala API.

I am not entirely sure which need weighs more: binary compatibility between
bug fix releases or a working API (small subset of it). Another aspect to
consider is how many people will migrate from 1.10.0 to 1.10.1 compared to
1.y to 1.10.1 with y <= 9. If the former is very small then one might make
a case for keeping the change.

What do the others think?

https://issues.apache.org/jira/browse/FLINK-13864
https://issues.apache.org/jira/browse/FLINK-16684

Cheers,
Till

On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]> wrote:

> We also noticed that and had to make an adjustment downstream.
>
> It would be good to mention this in the release notes (if that's not
> already the case).
>
> Thomas
>
>
> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]> wrote:
>
> > Hi Everyone,
> >
> > I realize I'm about 7 hours late but I just realized there is a breaking
> > API change in 1.10.1. FLINK-16684 changes the API of the streaming file
> > sink in a binary incompatible way. Since the release has been approved
> I'm
> > not sure the correct course of action but I wanted to bring this to the
> > communities attention.
> >
> > Seth
> >
> > https://issues.apache.org/jira/browse/FLINK-16684
> >
>

[Ufuk Celebi-2]

Thanks for the analysis Till.

1/ I think breaking binary compatibility is acceptable between minor
releases (1.10 -> 1.11) as you suggested since the API is marked
as @PublicEvolving.

2/ I'm quite torn about how to proceed with the 1.10.x patch release, but
slightly leaning towards the "pragmatic" solution of keeping the change and
adding a big warning to the 1.10.1 release announcement*. What do others
think? If we do want to revert the change and follow up with a 1.10.2
release we should do it _before_ we announce 1.10.1 publicly. Doing it
after 1.10.1 has been announced would only cause more friction for users.

– Ufuk

*I hope we don't lose user trust with this. I really would like users to be
able to upgrade to the latest patch release without hesitation.

On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <[hidden email]> wrote:

> Thanks for reporting this issue Seth. This is indeed a big problem.
>
> I looked into the problem and it seems we have the following situation:
>
> # 1.9.x --> 1.10.0
>
> There is an API breaking change between 1.9.x and 1.10 due FLINK-13864
> because it introduced another generic parameter. I expected only few people
> will be affected by this because one would have to store the builder in a
> variable.
>
> 1.9.x is binary compatible with 1.10.0.
>
> # 1.10.0 -> 1.10.1
>
> There is no API breaking change between 1.10.0 and 1.10.1.
>
> I changed the return type of the StreamingFileSink.forRowFormat and
> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder in
> FLINK-16684. This causes the java.lang.NoSuchMethodError and the binary
> incompatibility between 1.10.0 and 1.10.1. This is should not happen for
> bug fix releases.
>
> W/o FLINK-16684, the StreamingFileSink builders cannot be used with Flink's
> Scala API.
>
> # Options
>
> I think we should keep the fix for 1.11.0 and add a release note that we
> are violating binary compatibility between 1.10 and 1.11.
>
> Now the question is what to do with the 1.10.x branch:
>
> a) We can revert the change to re-establish binary compatibility between
> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This would also
> imply that we cannot use the StreamingFileSink builders with the Scala API.
>
> b) Keep the change and add a release note that our users need to recompile
> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
> StreamingFileSink builders with the Scala API.
>
> I am not entirely sure which need weighs more: binary compatibility between
> bug fix releases or a working API (small subset of it). Another aspect to
> consider is how many people will migrate from 1.10.0 to 1.10.1 compared to
> 1.y to 1.10.1 with y <= 9. If the former is very small then one might make
> a case for keeping the change.
>
> What do the others think?
>
> https://issues.apache.org/jira/browse/FLINK-13864
> https://issues.apache.org/jira/browse/FLINK-16684
>
> Cheers,
> Till
>
> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]> wrote:
>
> > We also noticed that and had to make an adjustment downstream.
> >
> > It would be good to mention this in the release notes (if that's not
> > already the case).
> >
> > Thomas
> >
> >
> > On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]>
> wrote:
> >
> > > Hi Everyone,
> > >
> > > I realize I'm about 7 hours late but I just realized there is a
> breaking
> > > API change in 1.10.1. FLINK-16684 changes the API of the streaming file
> > > sink in a binary incompatible way. Since the release has been approved
> > I'm
> > > not sure the correct course of action but I wanted to bring this to the
> > > communities attention.
> > >
> > > Seth
> > >
> > > https://issues.apache.org/jira/browse/FLINK-16684
> > >
> >
>

[Till Rohrmann]

A small addendum: The project currently enforces only binary compatibility
for classes which are annotated with @Public. The StreamingFileSink is
annotated with @PublicEvolving.

I am not sure whether the community properly defined what kind of
stability/compatibility guarantees we provide for @PublicEvolving classes.

We can derive two follow-up discussions from this:

1) Should the StreamingFileSink be made @Public? Should
other @PublicEvolving classes be promoted? What is the process here?

2) Should we enforce binary compatibility of @PublicEvolving classes
between bug fix releases and allow breaking changes between minor/major
releases?

Cheers,
Till

On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:

> Thanks for the analysis Till.
>
> 1/ I think breaking binary compatibility is acceptable between minor
> releases (1.10 -> 1.11) as you suggested since the API is marked
> as @PublicEvolving.
>
> 2/ I'm quite torn about how to proceed with the 1.10.x patch release, but
> slightly leaning towards the "pragmatic" solution of keeping the change and
> adding a big warning to the 1.10.1 release announcement*. What do others
> think? If we do want to revert the change and follow up with a 1.10.2
> release we should do it _before_ we announce 1.10.1 publicly. Doing it
> after 1.10.1 has been announced would only cause more friction for users.
>
> – Ufuk
>
> *I hope we don't lose user trust with this. I really would like users to be
> able to upgrade to the latest patch release without hesitation.
>
> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <[hidden email]>
> wrote:
>
> > Thanks for reporting this issue Seth. This is indeed a big problem.
> >
> > I looked into the problem and it seems we have the following situation:
> >
> > # 1.9.x --> 1.10.0
> >
> > There is an API breaking change between 1.9.x and 1.10 due FLINK-13864
> > because it introduced another generic parameter. I expected only few
> people
> > will be affected by this because one would have to store the builder in a
> > variable.
> >
> > 1.9.x is binary compatible with 1.10.0.
> >
> > # 1.10.0 -> 1.10.1
> >
> > There is no API breaking change between 1.10.0 and 1.10.1.
> >
> > I changed the return type of the StreamingFileSink.forRowFormat and
> > .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder in
> > FLINK-16684. This causes the java.lang.NoSuchMethodError and the binary
> > incompatibility between 1.10.0 and 1.10.1. This is should not happen for
> > bug fix releases.
> >
> > W/o FLINK-16684, the StreamingFileSink builders cannot be used with
> Flink's
> > Scala API.
> >
> > # Options
> >
> > I think we should keep the fix for 1.11.0 and add a release note that we
> > are violating binary compatibility between 1.10 and 1.11.
> >
> > Now the question is what to do with the 1.10.x branch:
> >
> > a) We can revert the change to re-establish binary compatibility between
> > 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This would
> also
> > imply that we cannot use the StreamingFileSink builders with the Scala
> API.
> >
> > b) Keep the change and add a release note that our users need to
> recompile
> > their jobs. This would allow Flink 1.10.x with x >= 1 users to use
> > StreamingFileSink builders with the Scala API.
> >
> > I am not entirely sure which need weighs more: binary compatibility
> between
> > bug fix releases or a working API (small subset of it). Another aspect to
> > consider is how many people will migrate from 1.10.0 to 1.10.1 compared
> to
> > 1.y to 1.10.1 with y <= 9. If the former is very small then one might
> make
> > a case for keeping the change.
> >
> > What do the others think?
> >
> > https://issues.apache.org/jira/browse/FLINK-13864
> > https://issues.apache.org/jira/browse/FLINK-16684
> >
> > Cheers,
> > Till
> >
> > On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]> wrote:
> >
> > > We also noticed that and had to make an adjustment downstream.
> > >
> > > It would be good to mention this in the release notes (if that's not
> > > already the case).
> > >
> > > Thomas
> > >
> > >
> > > On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]>
> > wrote:
> > >
> > > > Hi Everyone,
> > > >
> > > > I realize I'm about 7 hours late but I just realized there is a
> > breaking
> > > > API change in 1.10.1. FLINK-16684 changes the API of the streaming
> file
> > > > sink in a binary incompatible way. Since the release has been
> approved
> > > I'm
> > > > not sure the correct course of action but I wanted to bring this to
> the
> > > > communities attention.
> > > >
> > > > Seth
> > > >
> > > > https://issues.apache.org/jira/browse/FLINK-16684
> > > >
> > >
> >
>

[Chesnay Schepler-3]

IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0 announcement:

/"Flink 1.0.0 introduces interface stability annotations for API classes
and methods. Interfaces defined as //|@Public|//are guaranteed to remain
stable across all releases of the 1.x series. The
//|@PublicEvolving|//annotation marks API features that may be subject
to change in future versions."/

1) We don't have a process for promoting things to @Public because we
have actually never done that. Based on our current rules, it is an
addition to the public API (technically even more so than most FLIPs),
and hence should go through a FLIP and vote.
Ideally we evaluate existing @PublicEvolving API's for promotion on each
release.

2) Yes, I think this is an excellent idea; as Ufuk said users should not
be worried about upgrading to the next minor version. Not sure how easy
this is to implement; but it basically means either modifying or adding
new new japicmp execution when forking the release-X.Y branches.

On 13/05/2020 15:19, Till Rohrmann wrote:

> A small addendum: The project currently enforces only binary compatibility
> for classes which are annotated with @Public. The StreamingFileSink is
> annotated with @PublicEvolving.
>
> I am not sure whether the community properly defined what kind of
> stability/compatibility guarantees we provide for @PublicEvolving classes.
>
> We can derive two follow-up discussions from this:
>
> 1) Should the StreamingFileSink be made @Public? Should
> other @PublicEvolving classes be promoted? What is the process here?
>
> 2) Should we enforce binary compatibility of @PublicEvolving classes
> between bug fix releases and allow breaking changes between minor/major
> releases?
>
> Cheers,
> Till
>
> On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:
>
>> Thanks for the analysis Till.
>>
>> 1/ I think breaking binary compatibility is acceptable between minor
>> releases (1.10 -> 1.11) as you suggested since the API is marked
>> as @PublicEvolving.
>>
>> 2/ I'm quite torn about how to proceed with the 1.10.x patch release, but
>> slightly leaning towards the "pragmatic" solution of keeping the change and
>> adding a big warning to the 1.10.1 release announcement*. What do others
>> think? If we do want to revert the change and follow up with a 1.10.2
>> release we should do it _before_ we announce 1.10.1 publicly. Doing it
>> after 1.10.1 has been announced would only cause more friction for users.
>>
>> – Ufuk
>>
>> *I hope we don't lose user trust with this. I really would like users to be
>> able to upgrade to the latest patch release without hesitation.
>>
>> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <[hidden email]>
>> wrote:
>>
>>> Thanks for reporting this issue Seth. This is indeed a big problem.
>>>
>>> I looked into the problem and it seems we have the following situation:
>>>
>>> # 1.9.x --> 1.10.0
>>>
>>> There is an API breaking change between 1.9.x and 1.10 due FLINK-13864
>>> because it introduced another generic parameter. I expected only few
>> people
>>> will be affected by this because one would have to store the builder in a
>>> variable.
>>>
>>> 1.9.x is binary compatible with 1.10.0.
>>>
>>> # 1.10.0 -> 1.10.1
>>>
>>> There is no API breaking change between 1.10.0 and 1.10.1.
>>>
>>> I changed the return type of the StreamingFileSink.forRowFormat and
>>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder in
>>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the binary
>>> incompatibility between 1.10.0 and 1.10.1. This is should not happen for
>>> bug fix releases.
>>>
>>> W/o FLINK-16684, the StreamingFileSink builders cannot be used with
>> Flink's
>>> Scala API.
>>>
>>> # Options
>>>
>>> I think we should keep the fix for 1.11.0 and add a release note that we
>>> are violating binary compatibility between 1.10 and 1.11.
>>>
>>> Now the question is what to do with the 1.10.x branch:
>>>
>>> a) We can revert the change to re-establish binary compatibility between
>>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This would
>> also
>>> imply that we cannot use the StreamingFileSink builders with the Scala
>> API.
>>> b) Keep the change and add a release note that our users need to
>> recompile
>>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
>>> StreamingFileSink builders with the Scala API.
>>>
>>> I am not entirely sure which need weighs more: binary compatibility
>> between
>>> bug fix releases or a working API (small subset of it). Another aspect to
>>> consider is how many people will migrate from 1.10.0 to 1.10.1 compared
>> to
>>> 1.y to 1.10.1 with y <= 9. If the former is very small then one might
>> make
>>> a case for keeping the change.
>>>
>>> What do the others think?
>>>
>>> https://issues.apache.org/jira/browse/FLINK-13864
>>> https://issues.apache.org/jira/browse/FLINK-16684
>>>
>>> Cheers,
>>> Till
>>>
>>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]> wrote:
>>>
>>>> We also noticed that and had to make an adjustment downstream.
>>>>
>>>> It would be good to mention this in the release notes (if that's not
>>>> already the case).
>>>>
>>>> Thomas
>>>>
>>>>
>>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]>
>>> wrote:
>>>>> Hi Everyone,
>>>>>
>>>>> I realize I'm about 7 hours late but I just realized there is a
>>> breaking
>>>>> API change in 1.10.1. FLINK-16684 changes the API of the streaming
>> file
>>>>> sink in a binary incompatible way. Since the release has been
>> approved
>>>> I'm
>>>>> not sure the correct course of action but I wanted to bring this to
>> the
>>>>> communities attention.
>>>>>
>>>>> Seth
>>>>>
>>>>> https://issues.apache.org/jira/browse/FLINK-16684
>>>>>

[Seth Wiesman-4]

+1 Ufuk's pragmatic solution to update the release notes with a public
notice, I have commented on the release notes PR.

https://github.com/apache/flink-web/pull/330

Seth

On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <[hidden email]> wrote:

> IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0 announcement:
>
> /"Flink 1.0.0 introduces interface stability annotations for API classes
> and methods. Interfaces defined as //|@Public|//are guaranteed to remain
> stable across all releases of the 1.x series. The
> //|@PublicEvolving|//annotation marks API features that may be subject
> to change in future versions."/
>
> 1) We don't have a process for promoting things to @Public because we
> have actually never done that. Based on our current rules, it is an
> addition to the public API (technically even more so than most FLIPs),
> and hence should go through a FLIP and vote.
> Ideally we evaluate existing @PublicEvolving API's for promotion on each
> release.
>
> 2) Yes, I think this is an excellent idea; as Ufuk said users should not
> be worried about upgrading to the next minor version. Not sure how easy
> this is to implement; but it basically means either modifying or adding
> new new japicmp execution when forking the release-X.Y branches.
>
> On 13/05/2020 15:19, Till Rohrmann wrote:
> > A small addendum: The project currently enforces only binary
> compatibility
> > for classes which are annotated with @Public. The StreamingFileSink is
> > annotated with @PublicEvolving.
> >
> > I am not sure whether the community properly defined what kind of
> > stability/compatibility guarantees we provide for @PublicEvolving
> classes.
> >
> > We can derive two follow-up discussions from this:
> >
> > 1) Should the StreamingFileSink be made @Public? Should
> > other @PublicEvolving classes be promoted? What is the process here?
> >
> > 2) Should we enforce binary compatibility of @PublicEvolving classes
> > between bug fix releases and allow breaking changes between minor/major
> > releases?
> >
> > Cheers,
> > Till
> >
> > On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:
> >
> >> Thanks for the analysis Till.
> >>
> >> 1/ I think breaking binary compatibility is acceptable between minor
> >> releases (1.10 -> 1.11) as you suggested since the API is marked
> >> as @PublicEvolving.
> >>
> >> 2/ I'm quite torn about how to proceed with the 1.10.x patch release,
> but
> >> slightly leaning towards the "pragmatic" solution of keeping the change
> and
> >> adding a big warning to the 1.10.1 release announcement*. What do others
> >> think? If we do want to revert the change and follow up with a 1.10.2
> >> release we should do it _before_ we announce 1.10.1 publicly. Doing it
> >> after 1.10.1 has been announced would only cause more friction for
> users.
> >>
> >> – Ufuk
> >>
> >> *I hope we don't lose user trust with this. I really would like users
> to be
> >> able to upgrade to the latest patch release without hesitation.
> >>
> >> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <[hidden email]>
> >> wrote:
> >>
> >>> Thanks for reporting this issue Seth. This is indeed a big problem.
> >>>
> >>> I looked into the problem and it seems we have the following situation:
> >>>
> >>> # 1.9.x --> 1.10.0
> >>>
> >>> There is an API breaking change between 1.9.x and 1.10 due FLINK-13864
> >>> because it introduced another generic parameter. I expected only few
> >> people
> >>> will be affected by this because one would have to store the builder
> in a
> >>> variable.
> >>>
> >>> 1.9.x is binary compatible with 1.10.0.
> >>>
> >>> # 1.10.0 -> 1.10.1
> >>>
> >>> There is no API breaking change between 1.10.0 and 1.10.1.
> >>>
> >>> I changed the return type of the StreamingFileSink.forRowFormat and
> >>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder in
> >>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the binary
> >>> incompatibility between 1.10.0 and 1.10.1. This is should not happen
> for
> >>> bug fix releases.
> >>>
> >>> W/o FLINK-16684, the StreamingFileSink builders cannot be used with
> >> Flink's
> >>> Scala API.
> >>>
> >>> # Options
> >>>
> >>> I think we should keep the fix for 1.11.0 and add a release note that
> we
> >>> are violating binary compatibility between 1.10 and 1.11.
> >>>
> >>> Now the question is what to do with the 1.10.x branch:
> >>>
> >>> a) We can revert the change to re-establish binary compatibility
> between
> >>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This would
> >> also
> >>> imply that we cannot use the StreamingFileSink builders with the Scala
> >> API.
> >>> b) Keep the change and add a release note that our users need to
> >> recompile
> >>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
> >>> StreamingFileSink builders with the Scala API.
> >>>
> >>> I am not entirely sure which need weighs more: binary compatibility
> >> between
> >>> bug fix releases or a working API (small subset of it). Another aspect
> to
> >>> consider is how many people will migrate from 1.10.0 to 1.10.1 compared
> >> to
> >>> 1.y to 1.10.1 with y <= 9. If the former is very small then one might
> >> make
> >>> a case for keeping the change.
> >>>
> >>> What do the others think?
> >>>
> >>> https://issues.apache.org/jira/browse/FLINK-13864
> >>> https://issues.apache.org/jira/browse/FLINK-16684
> >>>
> >>> Cheers,
> >>> Till
> >>>
> >>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]> wrote:
> >>>
> >>>> We also noticed that and had to make an adjustment downstream.
> >>>>
> >>>> It would be good to mention this in the release notes (if that's not
> >>>> already the case).
> >>>>
> >>>> Thomas
> >>>>
> >>>>
> >>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]>
> >>> wrote:
> >>>>> Hi Everyone,
> >>>>>
> >>>>> I realize I'm about 7 hours late but I just realized there is a
> >>> breaking
> >>>>> API change in 1.10.1. FLINK-16684 changes the API of the streaming
> >> file
> >>>>> sink in a binary incompatible way. Since the release has been
> >> approved
> >>>> I'm
> >>>>> not sure the correct course of action but I wanted to bring this to
> >> the
> >>>>> communities attention.
> >>>>>
> >>>>> Seth
> >>>>>
> >>>>> https://issues.apache.org/jira/browse/FLINK-16684
> >>>>>
>
>

[Yu Li]

Thanks Seth, comment received, will add it into the release note.

Best Regards,
Yu


On Wed, 13 May 2020 at 22:54, Seth Wiesman <[hidden email]> wrote:

> +1 Ufuk's pragmatic solution to update the release notes with a public
> notice, I have commented on the release notes PR.
>
> https://github.com/apache/flink-web/pull/330
>
> Seth
>
> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <[hidden email]>
> wrote:
>
> > IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0
> announcement:
> >
> > /"Flink 1.0.0 introduces interface stability annotations for API classes
> > and methods. Interfaces defined as //|@Public|//are guaranteed to remain
> > stable across all releases of the 1.x series. The
> > //|@PublicEvolving|//annotation marks API features that may be subject
> > to change in future versions."/
> >
> > 1) We don't have a process for promoting things to @Public because we
> > have actually never done that. Based on our current rules, it is an
> > addition to the public API (technically even more so than most FLIPs),
> > and hence should go through a FLIP and vote.
> > Ideally we evaluate existing @PublicEvolving API's for promotion on each
> > release.
> >
> > 2) Yes, I think this is an excellent idea; as Ufuk said users should not
> > be worried about upgrading to the next minor version. Not sure how easy
> > this is to implement; but it basically means either modifying or adding
> > new new japicmp execution when forking the release-X.Y branches.
> >
> > On 13/05/2020 15:19, Till Rohrmann wrote:
> > > A small addendum: The project currently enforces only binary
> > compatibility
> > > for classes which are annotated with @Public. The StreamingFileSink is
> > > annotated with @PublicEvolving.
> > >
> > > I am not sure whether the community properly defined what kind of
> > > stability/compatibility guarantees we provide for @PublicEvolving
> > classes.
> > >
> > > We can derive two follow-up discussions from this:
> > >
> > > 1) Should the StreamingFileSink be made @Public? Should
> > > other @PublicEvolving classes be promoted? What is the process here?
> > >
> > > 2) Should we enforce binary compatibility of @PublicEvolving classes
> > > between bug fix releases and allow breaking changes between minor/major
> > > releases?
> > >
> > > Cheers,
> > > Till
> > >
> > > On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:
> > >
> > >> Thanks for the analysis Till.
> > >>
> > >> 1/ I think breaking binary compatibility is acceptable between minor
> > >> releases (1.10 -> 1.11) as you suggested since the API is marked
> > >> as @PublicEvolving.
> > >>
> > >> 2/ I'm quite torn about how to proceed with the 1.10.x patch release,
> > but
> > >> slightly leaning towards the "pragmatic" solution of keeping the
> change
> > and
> > >> adding a big warning to the 1.10.1 release announcement*. What do
> others
> > >> think? If we do want to revert the change and follow up with a 1.10.2
> > >> release we should do it _before_ we announce 1.10.1 publicly. Doing it
> > >> after 1.10.1 has been announced would only cause more friction for
> > users.
> > >>
> > >> – Ufuk
> > >>
> > >> *I hope we don't lose user trust with this. I really would like users
> > to be
> > >> able to upgrade to the latest patch release without hesitation.
> > >>
> > >> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <[hidden email]>
> > >> wrote:
> > >>
> > >>> Thanks for reporting this issue Seth. This is indeed a big problem.
> > >>>
> > >>> I looked into the problem and it seems we have the following
> situation:
> > >>>
> > >>> # 1.9.x --> 1.10.0
> > >>>
> > >>> There is an API breaking change between 1.9.x and 1.10 due
> FLINK-13864
> > >>> because it introduced another generic parameter. I expected only few
> > >> people
> > >>> will be affected by this because one would have to store the builder
> > in a
> > >>> variable.
> > >>>
> > >>> 1.9.x is binary compatible with 1.10.0.
> > >>>
> > >>> # 1.10.0 -> 1.10.1
> > >>>
> > >>> There is no API breaking change between 1.10.0 and 1.10.1.
> > >>>
> > >>> I changed the return type of the StreamingFileSink.forRowFormat and
> > >>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder in
> > >>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the
> binary
> > >>> incompatibility between 1.10.0 and 1.10.1. This is should not happen
> > for
> > >>> bug fix releases.
> > >>>
> > >>> W/o FLINK-16684, the StreamingFileSink builders cannot be used with
> > >> Flink's
> > >>> Scala API.
> > >>>
> > >>> # Options
> > >>>
> > >>> I think we should keep the fix for 1.11.0 and add a release note that
> > we
> > >>> are violating binary compatibility between 1.10 and 1.11.
> > >>>
> > >>> Now the question is what to do with the 1.10.x branch:
> > >>>
> > >>> a) We can revert the change to re-establish binary compatibility
> > between
> > >>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This
> would
> > >> also
> > >>> imply that we cannot use the StreamingFileSink builders with the
> Scala
> > >> API.
> > >>> b) Keep the change and add a release note that our users need to
> > >> recompile
> > >>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
> > >>> StreamingFileSink builders with the Scala API.
> > >>>
> > >>> I am not entirely sure which need weighs more: binary compatibility
> > >> between
> > >>> bug fix releases or a working API (small subset of it). Another
> aspect
> > to
> > >>> consider is how many people will migrate from 1.10.0 to 1.10.1
> compared
> > >> to
> > >>> 1.y to 1.10.1 with y <= 9. If the former is very small then one might
> > >> make
> > >>> a case for keeping the change.
> > >>>
> > >>> What do the others think?
> > >>>
> > >>> https://issues.apache.org/jira/browse/FLINK-13864
> > >>> https://issues.apache.org/jira/browse/FLINK-16684
> > >>>
> > >>> Cheers,
> > >>> Till
> > >>>
> > >>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]> wrote:
> > >>>
> > >>>> We also noticed that and had to make an adjustment downstream.
> > >>>>
> > >>>> It would be good to mention this in the release notes (if that's not
> > >>>> already the case).
> > >>>>
> > >>>> Thomas
> > >>>>
> > >>>>
> > >>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]>
> > >>> wrote:
> > >>>>> Hi Everyone,
> > >>>>>
> > >>>>> I realize I'm about 7 hours late but I just realized there is a
> > >>> breaking
> > >>>>> API change in 1.10.1. FLINK-16684 changes the API of the streaming
> > >> file
> > >>>>> sink in a binary incompatible way. Since the release has been
> > >> approved
> > >>>> I'm
> > >>>>> not sure the correct course of action but I wanted to bring this to
> > >> the
> > >>>>> communities attention.
> > >>>>>
> > >>>>> Seth
> > >>>>>
> > >>>>> https://issues.apache.org/jira/browse/FLINK-16684
> > >>>>>
> >
> >
>

[Thomas Weise]

+1 for the release note addition

Historically Flink X.Y.0 releases have been prone to issues like this. And
I suspect that many users would prefer to upgrade to X.Y.1 due to other
bugs that need to be shaken out.

For the future, it would be great to eliminate the possibility of
incompatible changes to user-facing classes for patch releases though.
Running japicmp as part of CI might be a good option.

Thanks,
Thomas

On Wed, May 13, 2020 at 7:54 AM Seth Wiesman <[hidden email]> wrote:

> +1 Ufuk's pragmatic solution to update the release notes with a public
> notice, I have commented on the release notes PR.
>
> https://github.com/apache/flink-web/pull/330
>
> Seth
>
> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <[hidden email]>
> wrote:
>
> > IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0
> announcement:
> >
> > /"Flink 1.0.0 introduces interface stability annotations for API classes
> > and methods. Interfaces defined as //|@Public|//are guaranteed to remain
> > stable across all releases of the 1.x series. The
> > //|@PublicEvolving|//annotation marks API features that may be subject
> > to change in future versions."/
> >
> > 1) We don't have a process for promoting things to @Public because we
> > have actually never done that. Based on our current rules, it is an
> > addition to the public API (technically even more so than most FLIPs),
> > and hence should go through a FLIP and vote.
> > Ideally we evaluate existing @PublicEvolving API's for promotion on each
> > release.
> >
> > 2) Yes, I think this is an excellent idea; as Ufuk said users should not
> > be worried about upgrading to the next minor version. Not sure how easy
> > this is to implement; but it basically means either modifying or adding
> > new new japicmp execution when forking the release-X.Y branches.
> >
> > On 13/05/2020 15:19, Till Rohrmann wrote:
> > > A small addendum: The project currently enforces only binary
> > compatibility
> > > for classes which are annotated with @Public. The StreamingFileSink is
> > > annotated with @PublicEvolving.
> > >
> > > I am not sure whether the community properly defined what kind of
> > > stability/compatibility guarantees we provide for @PublicEvolving
> > classes.
> > >
> > > We can derive two follow-up discussions from this:
> > >
> > > 1) Should the StreamingFileSink be made @Public? Should
> > > other @PublicEvolving classes be promoted? What is the process here?
> > >
> > > 2) Should we enforce binary compatibility of @PublicEvolving classes
> > > between bug fix releases and allow breaking changes between minor/major
> > > releases?
> > >
> > > Cheers,
> > > Till
> > >
> > > On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:
> > >
> > >> Thanks for the analysis Till.
> > >>
> > >> 1/ I think breaking binary compatibility is acceptable between minor
> > >> releases (1.10 -> 1.11) as you suggested since the API is marked
> > >> as @PublicEvolving.
> > >>
> > >> 2/ I'm quite torn about how to proceed with the 1.10.x patch release,
> > but
> > >> slightly leaning towards the "pragmatic" solution of keeping the
> change
> > and
> > >> adding a big warning to the 1.10.1 release announcement*. What do
> others
> > >> think? If we do want to revert the change and follow up with a 1.10.2
> > >> release we should do it _before_ we announce 1.10.1 publicly. Doing it
> > >> after 1.10.1 has been announced would only cause more friction for
> > users.
> > >>
> > >> – Ufuk
> > >>
> > >> *I hope we don't lose user trust with this. I really would like users
> > to be
> > >> able to upgrade to the latest patch release without hesitation.
> > >>
> > >> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <[hidden email]>
> > >> wrote:
> > >>
> > >>> Thanks for reporting this issue Seth. This is indeed a big problem.
> > >>>
> > >>> I looked into the problem and it seems we have the following
> situation:
> > >>>
> > >>> # 1.9.x --> 1.10.0
> > >>>
> > >>> There is an API breaking change between 1.9.x and 1.10 due
> FLINK-13864
> > >>> because it introduced another generic parameter. I expected only few
> > >> people
> > >>> will be affected by this because one would have to store the builder
> > in a
> > >>> variable.
> > >>>
> > >>> 1.9.x is binary compatible with 1.10.0.
> > >>>
> > >>> # 1.10.0 -> 1.10.1
> > >>>
> > >>> There is no API breaking change between 1.10.0 and 1.10.1.
> > >>>
> > >>> I changed the return type of the StreamingFileSink.forRowFormat and
> > >>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder in
> > >>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the
> binary
> > >>> incompatibility between 1.10.0 and 1.10.1. This is should not happen
> > for
> > >>> bug fix releases.
> > >>>
> > >>> W/o FLINK-16684, the StreamingFileSink builders cannot be used with
> > >> Flink's
> > >>> Scala API.
> > >>>
> > >>> # Options
> > >>>
> > >>> I think we should keep the fix for 1.11.0 and add a release note that
> > we
> > >>> are violating binary compatibility between 1.10 and 1.11.
> > >>>
> > >>> Now the question is what to do with the 1.10.x branch:
> > >>>
> > >>> a) We can revert the change to re-establish binary compatibility
> > between
> > >>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This
> would
> > >> also
> > >>> imply that we cannot use the StreamingFileSink builders with the
> Scala
> > >> API.
> > >>> b) Keep the change and add a release note that our users need to
> > >> recompile
> > >>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
> > >>> StreamingFileSink builders with the Scala API.
> > >>>
> > >>> I am not entirely sure which need weighs more: binary compatibility
> > >> between
> > >>> bug fix releases or a working API (small subset of it). Another
> aspect
> > to
> > >>> consider is how many people will migrate from 1.10.0 to 1.10.1
> compared
> > >> to
> > >>> 1.y to 1.10.1 with y <= 9. If the former is very small then one might
> > >> make
> > >>> a case for keeping the change.
> > >>>
> > >>> What do the others think?
> > >>>
> > >>> https://issues.apache.org/jira/browse/FLINK-13864
> > >>> https://issues.apache.org/jira/browse/FLINK-16684
> > >>>
> > >>> Cheers,
> > >>> Till
> > >>>
> > >>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]> wrote:
> > >>>
> > >>>> We also noticed that and had to make an adjustment downstream.
> > >>>>
> > >>>> It would be good to mention this in the release notes (if that's not
> > >>>> already the case).
> > >>>>
> > >>>> Thomas
> > >>>>
> > >>>>
> > >>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]>
> > >>> wrote:
> > >>>>> Hi Everyone,
> > >>>>>
> > >>>>> I realize I'm about 7 hours late but I just realized there is a
> > >>> breaking
> > >>>>> API change in 1.10.1. FLINK-16684 changes the API of the streaming
> > >> file
> > >>>>> sink in a binary incompatible way. Since the release has been
> > >> approved
> > >>>> I'm
> > >>>>> not sure the correct course of action but I wanted to bring this to
> > >> the
> > >>>>> communities attention.
> > >>>>>
> > >>>>> Seth
> > >>>>>
> > >>>>> https://issues.apache.org/jira/browse/FLINK-16684
> > >>>>>
> >
> >
>

[Chesnay Schepler-3]

We are running japicmp in CI; we just don't check anything that is
PublicEvolving.

On 13/05/2020 18:04, Thomas Weise wrote:

> +1 for the release note addition
>
> Historically Flink X.Y.0 releases have been prone to issues like this. And
> I suspect that many users would prefer to upgrade to X.Y.1 due to other
> bugs that need to be shaken out.
>
> For the future, it would be great to eliminate the possibility of
> incompatible changes to user-facing classes for patch releases though.
> Running japicmp as part of CI might be a good option.
>
> Thanks,
> Thomas
>
> On Wed, May 13, 2020 at 7:54 AM Seth Wiesman <[hidden email]> wrote:
>
>> +1 Ufuk's pragmatic solution to update the release notes with a public
>> notice, I have commented on the release notes PR.
>>
>> https://github.com/apache/flink-web/pull/330
>>
>> Seth
>>
>> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <[hidden email]>
>> wrote:
>>
>>> IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0
>> announcement:
>>> /"Flink 1.0.0 introduces interface stability annotations for API classes
>>> and methods. Interfaces defined as //|@Public|//are guaranteed to remain
>>> stable across all releases of the 1.x series. The
>>> //|@PublicEvolving|//annotation marks API features that may be subject
>>> to change in future versions."/
>>>
>>> 1) We don't have a process for promoting things to @Public because we
>>> have actually never done that. Based on our current rules, it is an
>>> addition to the public API (technically even more so than most FLIPs),
>>> and hence should go through a FLIP and vote.
>>> Ideally we evaluate existing @PublicEvolving API's for promotion on each
>>> release.
>>>
>>> 2) Yes, I think this is an excellent idea; as Ufuk said users should not
>>> be worried about upgrading to the next minor version. Not sure how easy
>>> this is to implement; but it basically means either modifying or adding
>>> new new japicmp execution when forking the release-X.Y branches.
>>>
>>> On 13/05/2020 15:19, Till Rohrmann wrote:
>>>> A small addendum: The project currently enforces only binary
>>> compatibility
>>>> for classes which are annotated with @Public. The StreamingFileSink is
>>>> annotated with @PublicEvolving.
>>>>
>>>> I am not sure whether the community properly defined what kind of
>>>> stability/compatibility guarantees we provide for @PublicEvolving
>>> classes.
>>>> We can derive two follow-up discussions from this:
>>>>
>>>> 1) Should the StreamingFileSink be made @Public? Should
>>>> other @PublicEvolving classes be promoted? What is the process here?
>>>>
>>>> 2) Should we enforce binary compatibility of @PublicEvolving classes
>>>> between bug fix releases and allow breaking changes between minor/major
>>>> releases?
>>>>
>>>> Cheers,
>>>> Till
>>>>
>>>> On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:
>>>>
>>>>> Thanks for the analysis Till.
>>>>>
>>>>> 1/ I think breaking binary compatibility is acceptable between minor
>>>>> releases (1.10 -> 1.11) as you suggested since the API is marked
>>>>> as @PublicEvolving.
>>>>>
>>>>> 2/ I'm quite torn about how to proceed with the 1.10.x patch release,
>>> but
>>>>> slightly leaning towards the "pragmatic" solution of keeping the
>> change
>>> and
>>>>> adding a big warning to the 1.10.1 release announcement*. What do
>> others
>>>>> think? If we do want to revert the change and follow up with a 1.10.2
>>>>> release we should do it _before_ we announce 1.10.1 publicly. Doing it
>>>>> after 1.10.1 has been announced would only cause more friction for
>>> users.
>>>>> – Ufuk
>>>>>
>>>>> *I hope we don't lose user trust with this. I really would like users
>>> to be
>>>>> able to upgrade to the latest patch release without hesitation.
>>>>>
>>>>> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <[hidden email]>
>>>>> wrote:
>>>>>
>>>>>> Thanks for reporting this issue Seth. This is indeed a big problem.
>>>>>>
>>>>>> I looked into the problem and it seems we have the following
>> situation:
>>>>>> # 1.9.x --> 1.10.0
>>>>>>
>>>>>> There is an API breaking change between 1.9.x and 1.10 due
>> FLINK-13864
>>>>>> because it introduced another generic parameter. I expected only few
>>>>> people
>>>>>> will be affected by this because one would have to store the builder
>>> in a
>>>>>> variable.
>>>>>>
>>>>>> 1.9.x is binary compatible with 1.10.0.
>>>>>>
>>>>>> # 1.10.0 -> 1.10.1
>>>>>>
>>>>>> There is no API breaking change between 1.10.0 and 1.10.1.
>>>>>>
>>>>>> I changed the return type of the StreamingFileSink.forRowFormat and
>>>>>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder in
>>>>>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the
>> binary
>>>>>> incompatibility between 1.10.0 and 1.10.1. This is should not happen
>>> for
>>>>>> bug fix releases.
>>>>>>
>>>>>> W/o FLINK-16684, the StreamingFileSink builders cannot be used with
>>>>> Flink's
>>>>>> Scala API.
>>>>>>
>>>>>> # Options
>>>>>>
>>>>>> I think we should keep the fix for 1.11.0 and add a release note that
>>> we
>>>>>> are violating binary compatibility between 1.10 and 1.11.
>>>>>>
>>>>>> Now the question is what to do with the 1.10.x branch:
>>>>>>
>>>>>> a) We can revert the change to re-establish binary compatibility
>>> between
>>>>>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This
>> would
>>>>> also
>>>>>> imply that we cannot use the StreamingFileSink builders with the
>> Scala
>>>>> API.
>>>>>> b) Keep the change and add a release note that our users need to
>>>>> recompile
>>>>>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
>>>>>> StreamingFileSink builders with the Scala API.
>>>>>>
>>>>>> I am not entirely sure which need weighs more: binary compatibility
>>>>> between
>>>>>> bug fix releases or a working API (small subset of it). Another
>> aspect
>>> to
>>>>>> consider is how many people will migrate from 1.10.0 to 1.10.1
>> compared
>>>>> to
>>>>>> 1.y to 1.10.1 with y <= 9. If the former is very small then one might
>>>>> make
>>>>>> a case for keeping the change.
>>>>>>
>>>>>> What do the others think?
>>>>>>
>>>>>> https://issues.apache.org/jira/browse/FLINK-13864
>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
>>>>>>
>>>>>> Cheers,
>>>>>> Till
>>>>>>
>>>>>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]> wrote:
>>>>>>
>>>>>>> We also noticed that and had to make an adjustment downstream.
>>>>>>>
>>>>>>> It would be good to mention this in the release notes (if that's not
>>>>>>> already the case).
>>>>>>>
>>>>>>> Thomas
>>>>>>>
>>>>>>>
>>>>>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]>
>>>>>> wrote:
>>>>>>>> Hi Everyone,
>>>>>>>>
>>>>>>>> I realize I'm about 7 hours late but I just realized there is a
>>>>>> breaking
>>>>>>>> API change in 1.10.1. FLINK-16684 changes the API of the streaming
>>>>> file
>>>>>>>> sink in a binary incompatible way. Since the release has been
>>>>> approved
>>>>>>> I'm
>>>>>>>> not sure the correct course of action but I wanted to bring this to
>>>>> the
>>>>>>>> communities attention.
>>>>>>>>
>>>>>>>> Seth
>>>>>>>>
>>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
>>>>>>>>
>>>

[Thomas Weise]

For release branches, it would be good to not exclude PublicEvolving from
checking, by default [1].

There may be instances where it is necessary to make exceptions to fix a
critical bug, but those could be whitelisted.

Similarly, it might be good to blacklist dependency changes for patch
releases, by default.

[1]
https://github.com/apache/flink/blob/e1e7d7f7ecc080c850a264021bf1b20e3d27d373/pom.xml#L1959

On Wed, May 13, 2020 at 9:16 AM Chesnay Schepler <[hidden email]> wrote:

> We are running japicmp in CI; we just don't check anything that is
> PublicEvolving.
>
> On 13/05/2020 18:04, Thomas Weise wrote:
> > +1 for the release note addition
> >
> > Historically Flink X.Y.0 releases have been prone to issues like this.
> And
> > I suspect that many users would prefer to upgrade to X.Y.1 due to other
> > bugs that need to be shaken out.
> >
> > For the future, it would be great to eliminate the possibility of
> > incompatible changes to user-facing classes for patch releases though.
> > Running japicmp as part of CI might be a good option.
> >
> > Thanks,
> > Thomas
> >
> > On Wed, May 13, 2020 at 7:54 AM Seth Wiesman <[hidden email]>
> wrote:
> >
> >> +1 Ufuk's pragmatic solution to update the release notes with a public
> >> notice, I have commented on the release notes PR.
> >>
> >> https://github.com/apache/flink-web/pull/330
> >>
> >> Seth
> >>
> >> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <[hidden email]>
> >> wrote:
> >>
> >>> IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0
> >> announcement:
> >>> /"Flink 1.0.0 introduces interface stability annotations for API
> classes
> >>> and methods. Interfaces defined as //|@Public|//are guaranteed to
> remain
> >>> stable across all releases of the 1.x series. The
> >>> //|@PublicEvolving|//annotation marks API features that may be subject
> >>> to change in future versions."/
> >>>
> >>> 1) We don't have a process for promoting things to @Public because we
> >>> have actually never done that. Based on our current rules, it is an
> >>> addition to the public API (technically even more so than most FLIPs),
> >>> and hence should go through a FLIP and vote.
> >>> Ideally we evaluate existing @PublicEvolving API's for promotion on
> each
> >>> release.
> >>>
> >>> 2) Yes, I think this is an excellent idea; as Ufuk said users should
> not
> >>> be worried about upgrading to the next minor version. Not sure how easy
> >>> this is to implement; but it basically means either modifying or adding
> >>> new new japicmp execution when forking the release-X.Y branches.
> >>>
> >>> On 13/05/2020 15:19, Till Rohrmann wrote:
> >>>> A small addendum: The project currently enforces only binary
> >>> compatibility
> >>>> for classes which are annotated with @Public. The StreamingFileSink is
> >>>> annotated with @PublicEvolving.
> >>>>
> >>>> I am not sure whether the community properly defined what kind of
> >>>> stability/compatibility guarantees we provide for @PublicEvolving
> >>> classes.
> >>>> We can derive two follow-up discussions from this:
> >>>>
> >>>> 1) Should the StreamingFileSink be made @Public? Should
> >>>> other @PublicEvolving classes be promoted? What is the process here?
> >>>>
> >>>> 2) Should we enforce binary compatibility of @PublicEvolving classes
> >>>> between bug fix releases and allow breaking changes between
> minor/major
> >>>> releases?
> >>>>
> >>>> Cheers,
> >>>> Till
> >>>>
> >>>> On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:
> >>>>
> >>>>> Thanks for the analysis Till.
> >>>>>
> >>>>> 1/ I think breaking binary compatibility is acceptable between minor
> >>>>> releases (1.10 -> 1.11) as you suggested since the API is marked
> >>>>> as @PublicEvolving.
> >>>>>
> >>>>> 2/ I'm quite torn about how to proceed with the 1.10.x patch release,
> >>> but
> >>>>> slightly leaning towards the "pragmatic" solution of keeping the
> >> change
> >>> and
> >>>>> adding a big warning to the 1.10.1 release announcement*. What do
> >> others
> >>>>> think? If we do want to revert the change and follow up with a 1.10.2
> >>>>> release we should do it _before_ we announce 1.10.1 publicly. Doing
> it
> >>>>> after 1.10.1 has been announced would only cause more friction for
> >>> users.
> >>>>> – Ufuk
> >>>>>
> >>>>> *I hope we don't lose user trust with this. I really would like users
> >>> to be
> >>>>> able to upgrade to the latest patch release without hesitation.
> >>>>>
> >>>>> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <[hidden email]
> >
> >>>>> wrote:
> >>>>>
> >>>>>> Thanks for reporting this issue Seth. This is indeed a big problem.
> >>>>>>
> >>>>>> I looked into the problem and it seems we have the following
> >> situation:
> >>>>>> # 1.9.x --> 1.10.0
> >>>>>>
> >>>>>> There is an API breaking change between 1.9.x and 1.10 due
> >> FLINK-13864
> >>>>>> because it introduced another generic parameter. I expected only few
> >>>>> people
> >>>>>> will be affected by this because one would have to store the builder
> >>> in a
> >>>>>> variable.
> >>>>>>
> >>>>>> 1.9.x is binary compatible with 1.10.0.
> >>>>>>
> >>>>>> # 1.10.0 -> 1.10.1
> >>>>>>
> >>>>>> There is no API breaking change between 1.10.0 and 1.10.1.
> >>>>>>
> >>>>>> I changed the return type of the StreamingFileSink.forRowFormat and
> >>>>>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder
> in
> >>>>>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the
> >> binary
> >>>>>> incompatibility between 1.10.0 and 1.10.1. This is should not happen
> >>> for
> >>>>>> bug fix releases.
> >>>>>>
> >>>>>> W/o FLINK-16684, the StreamingFileSink builders cannot be used with
> >>>>> Flink's
> >>>>>> Scala API.
> >>>>>>
> >>>>>> # Options
> >>>>>>
> >>>>>> I think we should keep the fix for 1.11.0 and add a release note
> that
> >>> we
> >>>>>> are violating binary compatibility between 1.10 and 1.11.
> >>>>>>
> >>>>>> Now the question is what to do with the 1.10.x branch:
> >>>>>>
> >>>>>> a) We can revert the change to re-establish binary compatibility
> >>> between
> >>>>>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This
> >> would
> >>>>> also
> >>>>>> imply that we cannot use the StreamingFileSink builders with the
> >> Scala
> >>>>> API.
> >>>>>> b) Keep the change and add a release note that our users need to
> >>>>> recompile
> >>>>>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
> >>>>>> StreamingFileSink builders with the Scala API.
> >>>>>>
> >>>>>> I am not entirely sure which need weighs more: binary compatibility
> >>>>> between
> >>>>>> bug fix releases or a working API (small subset of it). Another
> >> aspect
> >>> to
> >>>>>> consider is how many people will migrate from 1.10.0 to 1.10.1
> >> compared
> >>>>> to
> >>>>>> 1.y to 1.10.1 with y <= 9. If the former is very small then one
> might
> >>>>> make
> >>>>>> a case for keeping the change.
> >>>>>>
> >>>>>> What do the others think?
> >>>>>>
> >>>>>> https://issues.apache.org/jira/browse/FLINK-13864
> >>>>>> https://issues.apache.org/jira/browse/FLINK-16684
> >>>>>>
> >>>>>> Cheers,
> >>>>>> Till
> >>>>>>
> >>>>>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]>
> wrote:
> >>>>>>
> >>>>>>> We also noticed that and had to make an adjustment downstream.
> >>>>>>>
> >>>>>>> It would be good to mention this in the release notes (if that's
> not
> >>>>>>> already the case).
> >>>>>>>
> >>>>>>> Thomas
> >>>>>>>
> >>>>>>>
> >>>>>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]
> >
> >>>>>> wrote:
> >>>>>>>> Hi Everyone,
> >>>>>>>>
> >>>>>>>> I realize I'm about 7 hours late but I just realized there is a
> >>>>>> breaking
> >>>>>>>> API change in 1.10.1. FLINK-16684 changes the API of the streaming
> >>>>> file
> >>>>>>>> sink in a binary incompatible way. Since the release has been
> >>>>> approved
> >>>>>>> I'm
> >>>>>>>> not sure the correct course of action but I wanted to bring this
> to
> >>>>> the
> >>>>>>>> communities attention.
> >>>>>>>>
> >>>>>>>> Seth
> >>>>>>>>
> >>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
> >>>>>>>>
> >>>
>
>

[Yu Li]

+1 to improve and eliminate the possibility of incompatible changes to
user-facing classes for patch releases.

The PR for 1.10.1 release [1] is already updated, please kindly let me know
if any further comments.

I plan to merge the PR and officially announce the release in ~ half an
hour if no objections. Thanks.

Best Regards,
Yu


On Thu, 14 May 2020 at 00:30, Thomas Weise <[hidden email]> wrote:

> For release branches, it would be good to not exclude PublicEvolving from
> checking, by default [1].
>
> There may be instances where it is necessary to make exceptions to fix a
> critical bug, but those could be whitelisted.
>
> Similarly, it might be good to blacklist dependency changes for patch
> releases, by default.
>
> [1]
>
> https://github.com/apache/flink/blob/e1e7d7f7ecc080c850a264021bf1b20e3d27d373/pom.xml#L1959
>
> On Wed, May 13, 2020 at 9:16 AM Chesnay Schepler <[hidden email]>
> wrote:
>
> > We are running japicmp in CI; we just don't check anything that is
> > PublicEvolving.
> >
> > On 13/05/2020 18:04, Thomas Weise wrote:
> > > +1 for the release note addition
> > >
> > > Historically Flink X.Y.0 releases have been prone to issues like this.
> > And
> > > I suspect that many users would prefer to upgrade to X.Y.1 due to other
> > > bugs that need to be shaken out.
> > >
> > > For the future, it would be great to eliminate the possibility of
> > > incompatible changes to user-facing classes for patch releases though.
> > > Running japicmp as part of CI might be a good option.
> > >
> > > Thanks,
> > > Thomas
> > >
> > > On Wed, May 13, 2020 at 7:54 AM Seth Wiesman <[hidden email]>
> > wrote:
> > >
> > >> +1 Ufuk's pragmatic solution to update the release notes with a public
> > >> notice, I have commented on the release notes PR.
> > >>
> > >> https://github.com/apache/flink-web/pull/330
> > >>
> > >> Seth
> > >>
> > >> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <[hidden email]>
> > >> wrote:
> > >>
> > >>> IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0
> > >> announcement:
> > >>> /"Flink 1.0.0 introduces interface stability annotations for API
> > classes
> > >>> and methods. Interfaces defined as //|@Public|//are guaranteed to
> > remain
> > >>> stable across all releases of the 1.x series. The
> > >>> //|@PublicEvolving|//annotation marks API features that may be
> subject
> > >>> to change in future versions."/
> > >>>
> > >>> 1) We don't have a process for promoting things to @Public because we
> > >>> have actually never done that. Based on our current rules, it is an
> > >>> addition to the public API (technically even more so than most
> FLIPs),
> > >>> and hence should go through a FLIP and vote.
> > >>> Ideally we evaluate existing @PublicEvolving API's for promotion on
> > each
> > >>> release.
> > >>>
> > >>> 2) Yes, I think this is an excellent idea; as Ufuk said users should
> > not
> > >>> be worried about upgrading to the next minor version. Not sure how
> easy
> > >>> this is to implement; but it basically means either modifying or
> adding
> > >>> new new japicmp execution when forking the release-X.Y branches.
> > >>>
> > >>> On 13/05/2020 15:19, Till Rohrmann wrote:
> > >>>> A small addendum: The project currently enforces only binary
> > >>> compatibility
> > >>>> for classes which are annotated with @Public. The StreamingFileSink
> is
> > >>>> annotated with @PublicEvolving.
> > >>>>
> > >>>> I am not sure whether the community properly defined what kind of
> > >>>> stability/compatibility guarantees we provide for @PublicEvolving
> > >>> classes.
> > >>>> We can derive two follow-up discussions from this:
> > >>>>
> > >>>> 1) Should the StreamingFileSink be made @Public? Should
> > >>>> other @PublicEvolving classes be promoted? What is the process here?
> > >>>>
> > >>>> 2) Should we enforce binary compatibility of @PublicEvolving classes
> > >>>> between bug fix releases and allow breaking changes between
> > minor/major
> > >>>> releases?
> > >>>>
> > >>>> Cheers,
> > >>>> Till
> > >>>>
> > >>>> On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:
> > >>>>
> > >>>>> Thanks for the analysis Till.
> > >>>>>
> > >>>>> 1/ I think breaking binary compatibility is acceptable between
> minor
> > >>>>> releases (1.10 -> 1.11) as you suggested since the API is marked
> > >>>>> as @PublicEvolving.
> > >>>>>
> > >>>>> 2/ I'm quite torn about how to proceed with the 1.10.x patch
> release,
> > >>> but
> > >>>>> slightly leaning towards the "pragmatic" solution of keeping the
> > >> change
> > >>> and
> > >>>>> adding a big warning to the 1.10.1 release announcement*. What do
> > >> others
> > >>>>> think? If we do want to revert the change and follow up with a
> 1.10.2
> > >>>>> release we should do it _before_ we announce 1.10.1 publicly. Doing
> > it
> > >>>>> after 1.10.1 has been announced would only cause more friction for
> > >>> users.
> > >>>>> – Ufuk
> > >>>>>
> > >>>>> *I hope we don't lose user trust with this. I really would like
> users
> > >>> to be
> > >>>>> able to upgrade to the latest patch release without hesitation.
> > >>>>>
> > >>>>> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <
> [hidden email]
> > >
> > >>>>> wrote:
> > >>>>>
> > >>>>>> Thanks for reporting this issue Seth. This is indeed a big
> problem.
> > >>>>>>
> > >>>>>> I looked into the problem and it seems we have the following
> > >> situation:
> > >>>>>> # 1.9.x --> 1.10.0
> > >>>>>>
> > >>>>>> There is an API breaking change between 1.9.x and 1.10 due
> > >> FLINK-13864
> > >>>>>> because it introduced another generic parameter. I expected only
> few
> > >>>>> people
> > >>>>>> will be affected by this because one would have to store the
> builder
> > >>> in a
> > >>>>>> variable.
> > >>>>>>
> > >>>>>> 1.9.x is binary compatible with 1.10.0.
> > >>>>>>
> > >>>>>> # 1.10.0 -> 1.10.1
> > >>>>>>
> > >>>>>> There is no API breaking change between 1.10.0 and 1.10.1.
> > >>>>>>
> > >>>>>> I changed the return type of the StreamingFileSink.forRowFormat
> and
> > >>>>>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder
> > in
> > >>>>>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the
> > >> binary
> > >>>>>> incompatibility between 1.10.0 and 1.10.1. This is should not
> happen
> > >>> for
> > >>>>>> bug fix releases.
> > >>>>>>
> > >>>>>> W/o FLINK-16684, the StreamingFileSink builders cannot be used
> with
> > >>>>> Flink's
> > >>>>>> Scala API.
> > >>>>>>
> > >>>>>> # Options
> > >>>>>>
> > >>>>>> I think we should keep the fix for 1.11.0 and add a release note
> > that
> > >>> we
> > >>>>>> are violating binary compatibility between 1.10 and 1.11.
> > >>>>>>
> > >>>>>> Now the question is what to do with the 1.10.x branch:
> > >>>>>>
> > >>>>>> a) We can revert the change to re-establish binary compatibility
> > >>> between
> > >>>>>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This
> > >> would
> > >>>>> also
> > >>>>>> imply that we cannot use the StreamingFileSink builders with the
> > >> Scala
> > >>>>> API.
> > >>>>>> b) Keep the change and add a release note that our users need to
> > >>>>> recompile
> > >>>>>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
> > >>>>>> StreamingFileSink builders with the Scala API.
> > >>>>>>
> > >>>>>> I am not entirely sure which need weighs more: binary
> compatibility
> > >>>>> between
> > >>>>>> bug fix releases or a working API (small subset of it). Another
> > >> aspect
> > >>> to
> > >>>>>> consider is how many people will migrate from 1.10.0 to 1.10.1
> > >> compared
> > >>>>> to
> > >>>>>> 1.y to 1.10.1 with y <= 9. If the former is very small then one
> > might
> > >>>>> make
> > >>>>>> a case for keeping the change.
> > >>>>>>
> > >>>>>> What do the others think?
> > >>>>>>
> > >>>>>> https://issues.apache.org/jira/browse/FLINK-13864
> > >>>>>> https://issues.apache.org/jira/browse/FLINK-16684
> > >>>>>>
> > >>>>>> Cheers,
> > >>>>>> Till
> > >>>>>>
> > >>>>>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]>
> > wrote:
> > >>>>>>
> > >>>>>>> We also noticed that and had to make an adjustment downstream.
> > >>>>>>>
> > >>>>>>> It would be good to mention this in the release notes (if that's
> > not
> > >>>>>>> already the case).
> > >>>>>>>
> > >>>>>>> Thomas
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <
> [hidden email]
> > >
> > >>>>>> wrote:
> > >>>>>>>> Hi Everyone,
> > >>>>>>>>
> > >>>>>>>> I realize I'm about 7 hours late but I just realized there is a
> > >>>>>> breaking
> > >>>>>>>> API change in 1.10.1. FLINK-16684 changes the API of the
> streaming
> > >>>>> file
> > >>>>>>>> sink in a binary incompatible way. Since the release has been
> > >>>>> approved
> > >>>>>>> I'm
> > >>>>>>>> not sure the correct course of action but I wanted to bring this
> > to
> > >>>>> the
> > >>>>>>>> communities attention.
> > >>>>>>>>
> > >>>>>>>> Seth
> > >>>>>>>>
> > >>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
> > >>>>>>>>
> > >>>
> >
> >
>

[Piotr Nowojski-3]

Hi,

> For the future, it would be great to eliminate the possibility of
> incompatible changes to user-facing classes for patch releases though.
> Running japicmp as part of CI might be a good option.

I guess the whole point of PublicEvolving classes is to allow us for the breaking compatibility changes.

Piotrek

> On 13 May 2020, at 18:29, Thomas Weise <[hidden email]> wrote:
>
> For release branches, it would be good to not exclude PublicEvolving from
> checking, by default [1].
>
> There may be instances where it is necessary to make exceptions to fix a
> critical bug, but those could be whitelisted.
>
> Similarly, it might be good to blacklist dependency changes for patch
> releases, by default.
>
> [1]
> https://github.com/apache/flink/blob/e1e7d7f7ecc080c850a264021bf1b20e3d27d373/pom.xml#L1959
>
> On Wed, May 13, 2020 at 9:16 AM Chesnay Schepler <[hidden email]> wrote:
>
>> We are running japicmp in CI; we just don't check anything that is
>> PublicEvolving.
>>
>> On 13/05/2020 18:04, Thomas Weise wrote:
>>> +1 for the release note addition
>>>
>>> Historically Flink X.Y.0 releases have been prone to issues like this.
>> And
>>> I suspect that many users would prefer to upgrade to X.Y.1 due to other
>>> bugs that need to be shaken out.
>>>
>>> For the future, it would be great to eliminate the possibility of
>>> incompatible changes to user-facing classes for patch releases though.
>>> Running japicmp as part of CI might be a good option.
>>>
>>> Thanks,
>>> Thomas
>>>
>>> On Wed, May 13, 2020 at 7:54 AM Seth Wiesman <[hidden email]>
>> wrote:
>>>
>>>> +1 Ufuk's pragmatic solution to update the release notes with a public
>>>> notice, I have commented on the release notes PR.
>>>>
>>>> https://github.com/apache/flink-web/pull/330
>>>>
>>>> Seth
>>>>
>>>> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <[hidden email]>
>>>> wrote:
>>>>
>>>>> IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0
>>>> announcement:
>>>>> /"Flink 1.0.0 introduces interface stability annotations for API
>> classes
>>>>> and methods. Interfaces defined as //|@Public|//are guaranteed to
>> remain
>>>>> stable across all releases of the 1.x series. The
>>>>> //|@PublicEvolving|//annotation marks API features that may be subject
>>>>> to change in future versions."/
>>>>>
>>>>> 1) We don't have a process for promoting things to @Public because we
>>>>> have actually never done that. Based on our current rules, it is an
>>>>> addition to the public API (technically even more so than most FLIPs),
>>>>> and hence should go through a FLIP and vote.
>>>>> Ideally we evaluate existing @PublicEvolving API's for promotion on
>> each
>>>>> release.
>>>>>
>>>>> 2) Yes, I think this is an excellent idea; as Ufuk said users should
>> not
>>>>> be worried about upgrading to the next minor version. Not sure how easy
>>>>> this is to implement; but it basically means either modifying or adding
>>>>> new new japicmp execution when forking the release-X.Y branches.
>>>>>
>>>>> On 13/05/2020 15:19, Till Rohrmann wrote:
>>>>>> A small addendum: The project currently enforces only binary
>>>>> compatibility
>>>>>> for classes which are annotated with @Public. The StreamingFileSink is
>>>>>> annotated with @PublicEvolving.
>>>>>>
>>>>>> I am not sure whether the community properly defined what kind of
>>>>>> stability/compatibility guarantees we provide for @PublicEvolving
>>>>> classes.
>>>>>> We can derive two follow-up discussions from this:
>>>>>>
>>>>>> 1) Should the StreamingFileSink be made @Public? Should
>>>>>> other @PublicEvolving classes be promoted? What is the process here?
>>>>>>
>>>>>> 2) Should we enforce binary compatibility of @PublicEvolving classes
>>>>>> between bug fix releases and allow breaking changes between
>> minor/major
>>>>>> releases?
>>>>>>
>>>>>> Cheers,
>>>>>> Till
>>>>>>
>>>>>> On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:
>>>>>>
>>>>>>> Thanks for the analysis Till.
>>>>>>>
>>>>>>> 1/ I think breaking binary compatibility is acceptable between minor
>>>>>>> releases (1.10 -> 1.11) as you suggested since the API is marked
>>>>>>> as @PublicEvolving.
>>>>>>>
>>>>>>> 2/ I'm quite torn about how to proceed with the 1.10.x patch release,
>>>>> but
>>>>>>> slightly leaning towards the "pragmatic" solution of keeping the
>>>> change
>>>>> and
>>>>>>> adding a big warning to the 1.10.1 release announcement*. What do
>>>> others
>>>>>>> think? If we do want to revert the change and follow up with a 1.10.2
>>>>>>> release we should do it _before_ we announce 1.10.1 publicly. Doing
>> it
>>>>>>> after 1.10.1 has been announced would only cause more friction for
>>>>> users.
>>>>>>> – Ufuk
>>>>>>>
>>>>>>> *I hope we don't lose user trust with this. I really would like users
>>>>> to be
>>>>>>> able to upgrade to the latest patch release without hesitation.
>>>>>>>
>>>>>>> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <[hidden email]
>>>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Thanks for reporting this issue Seth. This is indeed a big problem.
>>>>>>>>
>>>>>>>> I looked into the problem and it seems we have the following
>>>> situation:
>>>>>>>> # 1.9.x --> 1.10.0
>>>>>>>>
>>>>>>>> There is an API breaking change between 1.9.x and 1.10 due
>>>> FLINK-13864
>>>>>>>> because it introduced another generic parameter. I expected only few
>>>>>>> people
>>>>>>>> will be affected by this because one would have to store the builder
>>>>> in a
>>>>>>>> variable.
>>>>>>>>
>>>>>>>> 1.9.x is binary compatible with 1.10.0.
>>>>>>>>
>>>>>>>> # 1.10.0 -> 1.10.1
>>>>>>>>
>>>>>>>> There is no API breaking change between 1.10.0 and 1.10.1.
>>>>>>>>
>>>>>>>> I changed the return type of the StreamingFileSink.forRowFormat and
>>>>>>>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder
>> in
>>>>>>>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the
>>>> binary
>>>>>>>> incompatibility between 1.10.0 and 1.10.1. This is should not happen
>>>>> for
>>>>>>>> bug fix releases.
>>>>>>>>
>>>>>>>> W/o FLINK-16684, the StreamingFileSink builders cannot be used with
>>>>>>> Flink's
>>>>>>>> Scala API.
>>>>>>>>
>>>>>>>> # Options
>>>>>>>>
>>>>>>>> I think we should keep the fix for 1.11.0 and add a release note
>> that
>>>>> we
>>>>>>>> are violating binary compatibility between 1.10 and 1.11.
>>>>>>>>
>>>>>>>> Now the question is what to do with the 1.10.x branch:
>>>>>>>>
>>>>>>>> a) We can revert the change to re-establish binary compatibility
>>>>> between
>>>>>>>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This
>>>> would
>>>>>>> also
>>>>>>>> imply that we cannot use the StreamingFileSink builders with the
>>>> Scala
>>>>>>> API.
>>>>>>>> b) Keep the change and add a release note that our users need to
>>>>>>> recompile
>>>>>>>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
>>>>>>>> StreamingFileSink builders with the Scala API.
>>>>>>>>
>>>>>>>> I am not entirely sure which need weighs more: binary compatibility
>>>>>>> between
>>>>>>>> bug fix releases or a working API (small subset of it). Another
>>>> aspect
>>>>> to
>>>>>>>> consider is how many people will migrate from 1.10.0 to 1.10.1
>>>> compared
>>>>>>> to
>>>>>>>> 1.y to 1.10.1 with y <= 9. If the former is very small then one
>> might
>>>>>>> make
>>>>>>>> a case for keeping the change.
>>>>>>>>
>>>>>>>> What do the others think?
>>>>>>>>
>>>>>>>> https://issues.apache.org/jira/browse/FLINK-13864
>>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Till
>>>>>>>>
>>>>>>>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]>
>> wrote:
>>>>>>>>
>>>>>>>>> We also noticed that and had to make an adjustment downstream.
>>>>>>>>>
>>>>>>>>> It would be good to mention this in the release notes (if that's
>> not
>>>>>>>>> already the case).
>>>>>>>>>
>>>>>>>>> Thomas
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <[hidden email]
>>>
>>>>>>>> wrote:
>>>>>>>>>> Hi Everyone,
>>>>>>>>>>
>>>>>>>>>> I realize I'm about 7 hours late but I just realized there is a
>>>>>>>> breaking
>>>>>>>>>> API change in 1.10.1. FLINK-16684 changes the API of the streaming
>>>>>>> file
>>>>>>>>>> sink in a binary incompatible way. Since the release has been
>>>>>>> approved
>>>>>>>>> I'm
>>>>>>>>>> not sure the correct course of action but I wanted to bring this
>> to
>>>>>>> the
>>>>>>>>>> communities attention.
>>>>>>>>>>
>>>>>>>>>> Seth
>>>>>>>>>>
>>>>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
>>>>>>>>>>
>>>>>
>>
>>

[Thomas Weise]

On Wed, May 13, 2020 at 1:02 PM Piotr Nowojski <[hidden email]> wrote:

> Hi,
>
> > For the future, it would be great to eliminate the possibility of
> > incompatible changes to user-facing classes for patch releases though.
> > Running japicmp as part of CI might be a good option.
>
> I guess the whole point of PublicEvolving classes is to allow us for the
> breaking compatibility changes.
>
>

The question here is whether such changes should occur in *patch* releases.
I suspect most users would prefer stronger guarantees.

Another example of "breaking" change that affects Beam (though class
OptimizerPlanEnvironment isn't covered by annotation and it only affects
test code):

https://github.com/apache/beam/pull/11683

Piotrek

>
> > On 13 May 2020, at 18:29, Thomas Weise <[hidden email]> wrote:
> >
> > For release branches, it would be good to not exclude PublicEvolving from
> > checking, by default [1].
> >
> > There may be instances where it is necessary to make exceptions to fix a
> > critical bug, but those could be whitelisted.
> >
> > Similarly, it might be good to blacklist dependency changes for patch
> > releases, by default.
> >
> > [1]
> >
> https://github.com/apache/flink/blob/e1e7d7f7ecc080c850a264021bf1b20e3d27d373/pom.xml#L1959
> >
> > On Wed, May 13, 2020 at 9:16 AM Chesnay Schepler <[hidden email]>
> wrote:
> >
> >> We are running japicmp in CI; we just don't check anything that is
> >> PublicEvolving.
> >>
> >> On 13/05/2020 18:04, Thomas Weise wrote:
> >>> +1 for the release note addition
> >>>
> >>> Historically Flink X.Y.0 releases have been prone to issues like this.
> >> And
> >>> I suspect that many users would prefer to upgrade to X.Y.1 due to other
> >>> bugs that need to be shaken out.
> >>>
> >>> For the future, it would be great to eliminate the possibility of
> >>> incompatible changes to user-facing classes for patch releases though.
> >>> Running japicmp as part of CI might be a good option.
> >>>
> >>> Thanks,
> >>> Thomas
> >>>
> >>> On Wed, May 13, 2020 at 7:54 AM Seth Wiesman <[hidden email]>
> >> wrote:
> >>>
> >>>> +1 Ufuk's pragmatic solution to update the release notes with a public
> >>>> notice, I have commented on the release notes PR.
> >>>>
> >>>> https://github.com/apache/flink-web/pull/330
> >>>>
> >>>> Seth
> >>>>
> >>>> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <[hidden email]>
> >>>> wrote:
> >>>>
> >>>>> IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0
> >>>> announcement:
> >>>>> /"Flink 1.0.0 introduces interface stability annotations for API
> >> classes
> >>>>> and methods. Interfaces defined as //|@Public|//are guaranteed to
> >> remain
> >>>>> stable across all releases of the 1.x series. The
> >>>>> //|@PublicEvolving|//annotation marks API features that may be
> subject
> >>>>> to change in future versions."/
> >>>>>
> >>>>> 1) We don't have a process for promoting things to @Public because we
> >>>>> have actually never done that. Based on our current rules, it is an
> >>>>> addition to the public API (technically even more so than most
> FLIPs),
> >>>>> and hence should go through a FLIP and vote.
> >>>>> Ideally we evaluate existing @PublicEvolving API's for promotion on
> >> each
> >>>>> release.
> >>>>>
> >>>>> 2) Yes, I think this is an excellent idea; as Ufuk said users should
> >> not
> >>>>> be worried about upgrading to the next minor version. Not sure how
> easy
> >>>>> this is to implement; but it basically means either modifying or
> adding
> >>>>> new new japicmp execution when forking the release-X.Y branches.
> >>>>>
> >>>>> On 13/05/2020 15:19, Till Rohrmann wrote:
> >>>>>> A small addendum: The project currently enforces only binary
> >>>>> compatibility
> >>>>>> for classes which are annotated with @Public. The StreamingFileSink
> is
> >>>>>> annotated with @PublicEvolving.
> >>>>>>
> >>>>>> I am not sure whether the community properly defined what kind of
> >>>>>> stability/compatibility guarantees we provide for @PublicEvolving
> >>>>> classes.
> >>>>>> We can derive two follow-up discussions from this:
> >>>>>>
> >>>>>> 1) Should the StreamingFileSink be made @Public? Should
> >>>>>> other @PublicEvolving classes be promoted? What is the process here?
> >>>>>>
> >>>>>> 2) Should we enforce binary compatibility of @PublicEvolving classes
> >>>>>> between bug fix releases and allow breaking changes between
> >> minor/major
> >>>>>> releases?
> >>>>>>
> >>>>>> Cheers,
> >>>>>> Till
> >>>>>>
> >>>>>> On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]> wrote:
> >>>>>>
> >>>>>>> Thanks for the analysis Till.
> >>>>>>>
> >>>>>>> 1/ I think breaking binary compatibility is acceptable between
> minor
> >>>>>>> releases (1.10 -> 1.11) as you suggested since the API is marked
> >>>>>>> as @PublicEvolving.
> >>>>>>>
> >>>>>>> 2/ I'm quite torn about how to proceed with the 1.10.x patch
> release,
> >>>>> but
> >>>>>>> slightly leaning towards the "pragmatic" solution of keeping the
> >>>> change
> >>>>> and
> >>>>>>> adding a big warning to the 1.10.1 release announcement*. What do
> >>>> others
> >>>>>>> think? If we do want to revert the change and follow up with a
> 1.10.2
> >>>>>>> release we should do it _before_ we announce 1.10.1 publicly. Doing
> >> it
> >>>>>>> after 1.10.1 has been announced would only cause more friction for
> >>>>> users.
> >>>>>>> – Ufuk
> >>>>>>>
> >>>>>>> *I hope we don't lose user trust with this. I really would like
> users
> >>>>> to be
> >>>>>>> able to upgrade to the latest patch release without hesitation.
> >>>>>>>
> >>>>>>> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <
> [hidden email]
> >>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> Thanks for reporting this issue Seth. This is indeed a big
> problem.
> >>>>>>>>
> >>>>>>>> I looked into the problem and it seems we have the following
> >>>> situation:
> >>>>>>>> # 1.9.x --> 1.10.0
> >>>>>>>>
> >>>>>>>> There is an API breaking change between 1.9.x and 1.10 due
> >>>> FLINK-13864
> >>>>>>>> because it introduced another generic parameter. I expected only
> few
> >>>>>>> people
> >>>>>>>> will be affected by this because one would have to store the
> builder
> >>>>> in a
> >>>>>>>> variable.
> >>>>>>>>
> >>>>>>>> 1.9.x is binary compatible with 1.10.0.
> >>>>>>>>
> >>>>>>>> # 1.10.0 -> 1.10.1
> >>>>>>>>
> >>>>>>>> There is no API breaking change between 1.10.0 and 1.10.1.
> >>>>>>>>
> >>>>>>>> I changed the return type of the StreamingFileSink.forRowFormat
> and
> >>>>>>>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder
> >> in
> >>>>>>>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the
> >>>> binary
> >>>>>>>> incompatibility between 1.10.0 and 1.10.1. This is should not
> happen
> >>>>> for
> >>>>>>>> bug fix releases.
> >>>>>>>>
> >>>>>>>> W/o FLINK-16684, the StreamingFileSink builders cannot be used
> with
> >>>>>>> Flink's
> >>>>>>>> Scala API.
> >>>>>>>>
> >>>>>>>> # Options
> >>>>>>>>
> >>>>>>>> I think we should keep the fix for 1.11.0 and add a release note
> >> that
> >>>>> we
> >>>>>>>> are violating binary compatibility between 1.10 and 1.11.
> >>>>>>>>
> >>>>>>>> Now the question is what to do with the 1.10.x branch:
> >>>>>>>>
> >>>>>>>> a) We can revert the change to re-establish binary compatibility
> >>>>> between
> >>>>>>>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This
> >>>> would
> >>>>>>> also
> >>>>>>>> imply that we cannot use the StreamingFileSink builders with the
> >>>> Scala
> >>>>>>> API.
> >>>>>>>> b) Keep the change and add a release note that our users need to
> >>>>>>> recompile
> >>>>>>>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use
> >>>>>>>> StreamingFileSink builders with the Scala API.
> >>>>>>>>
> >>>>>>>> I am not entirely sure which need weighs more: binary
> compatibility
> >>>>>>> between
> >>>>>>>> bug fix releases or a working API (small subset of it). Another
> >>>> aspect
> >>>>> to
> >>>>>>>> consider is how many people will migrate from 1.10.0 to 1.10.1
> >>>> compared
> >>>>>>> to
> >>>>>>>> 1.y to 1.10.1 with y <= 9. If the former is very small then one
> >> might
> >>>>>>> make
> >>>>>>>> a case for keeping the change.
> >>>>>>>>
> >>>>>>>> What do the others think?
> >>>>>>>>
> >>>>>>>> https://issues.apache.org/jira/browse/FLINK-13864
> >>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
> >>>>>>>>
> >>>>>>>> Cheers,
> >>>>>>>> Till
> >>>>>>>>
> >>>>>>>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]>
> >> wrote:
> >>>>>>>>
> >>>>>>>>> We also noticed that and had to make an adjustment downstream.
> >>>>>>>>>
> >>>>>>>>> It would be good to mention this in the release notes (if that's
> >> not
> >>>>>>>>> already the case).
> >>>>>>>>>
> >>>>>>>>> Thomas
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <
> [hidden email]
> >>>
> >>>>>>>> wrote:
> >>>>>>>>>> Hi Everyone,
> >>>>>>>>>>
> >>>>>>>>>> I realize I'm about 7 hours late but I just realized there is a
> >>>>>>>> breaking
> >>>>>>>>>> API change in 1.10.1. FLINK-16684 changes the API of the
> streaming
> >>>>>>> file
> >>>>>>>>>> sink in a binary incompatible way. Since the release has been
> >>>>>>> approved
> >>>>>>>>> I'm
> >>>>>>>>>> not sure the correct course of action but I wanted to bring this
> >> to
> >>>>>>> the
> >>>>>>>>>> communities attention.
> >>>>>>>>>>
> >>>>>>>>>> Seth
> >>>>>>>>>>
> >>>>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
> >>>>>>>>>>
> >>>>>
> >>
> >>
>
>

[Till Rohrmann]

I agree with the pragmatic solution.

Concerning the stability guarantees I will start a separate discussion
thread whether to provide stricter guarantees or not.

Cheers,
Till

On Wed, May 13, 2020 at 10:13 PM Thomas Weise <[hidden email]> wrote:

> On Wed, May 13, 2020 at 1:02 PM Piotr Nowojski <[hidden email]>
> wrote:
>
> > Hi,
> >
> > > For the future, it would be great to eliminate the possibility of
> > > incompatible changes to user-facing classes for patch releases though.
> > > Running japicmp as part of CI might be a good option.
> >
> > I guess the whole point of PublicEvolving classes is to allow us for the
> > breaking compatibility changes.
> >
> >
> The question here is whether such changes should occur in *patch* releases.
> I suspect most users would prefer stronger guarantees.
>
> Another example of "breaking" change that affects Beam (though class
> OptimizerPlanEnvironment isn't covered by annotation and it only affects
> test code):
>
> https://github.com/apache/beam/pull/11683
>
> Piotrek
> >
> > > On 13 May 2020, at 18:29, Thomas Weise <[hidden email]> wrote:
> > >
> > > For release branches, it would be good to not exclude PublicEvolving
> from
> > > checking, by default [1].
> > >
> > > There may be instances where it is necessary to make exceptions to fix
> a
> > > critical bug, but those could be whitelisted.
> > >
> > > Similarly, it might be good to blacklist dependency changes for patch
> > > releases, by default.
> > >
> > > [1]
> > >
> >
> https://github.com/apache/flink/blob/e1e7d7f7ecc080c850a264021bf1b20e3d27d373/pom.xml#L1959
> > >
> > > On Wed, May 13, 2020 at 9:16 AM Chesnay Schepler <[hidden email]>
> > wrote:
> > >
> > >> We are running japicmp in CI; we just don't check anything that is
> > >> PublicEvolving.
> > >>
> > >> On 13/05/2020 18:04, Thomas Weise wrote:
> > >>> +1 for the release note addition
> > >>>
> > >>> Historically Flink X.Y.0 releases have been prone to issues like
> this.
> > >> And
> > >>> I suspect that many users would prefer to upgrade to X.Y.1 due to
> other
> > >>> bugs that need to be shaken out.
> > >>>
> > >>> For the future, it would be great to eliminate the possibility of
> > >>> incompatible changes to user-facing classes for patch releases
> though.
> > >>> Running japicmp as part of CI might be a good option.
> > >>>
> > >>> Thanks,
> > >>> Thomas
> > >>>
> > >>> On Wed, May 13, 2020 at 7:54 AM Seth Wiesman <[hidden email]>
> > >> wrote:
> > >>>
> > >>>> +1 Ufuk's pragmatic solution to update the release notes with a
> public
> > >>>> notice, I have commented on the release notes PR.
> > >>>>
> > >>>> https://github.com/apache/flink-web/pull/330
> > >>>>
> > >>>> Seth
> > >>>>
> > >>>> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <
> [hidden email]>
> > >>>> wrote:
> > >>>>
> > >>>>> IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0
> > >>>> announcement:
> > >>>>> /"Flink 1.0.0 introduces interface stability annotations for API
> > >> classes
> > >>>>> and methods. Interfaces defined as //|@Public|//are guaranteed to
> > >> remain
> > >>>>> stable across all releases of the 1.x series. The
> > >>>>> //|@PublicEvolving|//annotation marks API features that may be
> > subject
> > >>>>> to change in future versions."/
> > >>>>>
> > >>>>> 1) We don't have a process for promoting things to @Public because
> we
> > >>>>> have actually never done that. Based on our current rules, it is an
> > >>>>> addition to the public API (technically even more so than most
> > FLIPs),
> > >>>>> and hence should go through a FLIP and vote.
> > >>>>> Ideally we evaluate existing @PublicEvolving API's for promotion on
> > >> each
> > >>>>> release.
> > >>>>>
> > >>>>> 2) Yes, I think this is an excellent idea; as Ufuk said users
> should
> > >> not
> > >>>>> be worried about upgrading to the next minor version. Not sure how
> > easy
> > >>>>> this is to implement; but it basically means either modifying or
> > adding
> > >>>>> new new japicmp execution when forking the release-X.Y branches.
> > >>>>>
> > >>>>> On 13/05/2020 15:19, Till Rohrmann wrote:
> > >>>>>> A small addendum: The project currently enforces only binary
> > >>>>> compatibility
> > >>>>>> for classes which are annotated with @Public. The
> StreamingFileSink
> > is
> > >>>>>> annotated with @PublicEvolving.
> > >>>>>>
> > >>>>>> I am not sure whether the community properly defined what kind of
> > >>>>>> stability/compatibility guarantees we provide for @PublicEvolving
> > >>>>> classes.
> > >>>>>> We can derive two follow-up discussions from this:
> > >>>>>>
> > >>>>>> 1) Should the StreamingFileSink be made @Public? Should
> > >>>>>> other @PublicEvolving classes be promoted? What is the process
> here?
> > >>>>>>
> > >>>>>> 2) Should we enforce binary compatibility of @PublicEvolving
> classes
> > >>>>>> between bug fix releases and allow breaking changes between
> > >> minor/major
> > >>>>>> releases?
> > >>>>>>
> > >>>>>> Cheers,
> > >>>>>> Till
> > >>>>>>
> > >>>>>> On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]>
> wrote:
> > >>>>>>
> > >>>>>>> Thanks for the analysis Till.
> > >>>>>>>
> > >>>>>>> 1/ I think breaking binary compatibility is acceptable between
> > minor
> > >>>>>>> releases (1.10 -> 1.11) as you suggested since the API is marked
> > >>>>>>> as @PublicEvolving.
> > >>>>>>>
> > >>>>>>> 2/ I'm quite torn about how to proceed with the 1.10.x patch
> > release,
> > >>>>> but
> > >>>>>>> slightly leaning towards the "pragmatic" solution of keeping the
> > >>>> change
> > >>>>> and
> > >>>>>>> adding a big warning to the 1.10.1 release announcement*. What do
> > >>>> others
> > >>>>>>> think? If we do want to revert the change and follow up with a
> > 1.10.2
> > >>>>>>> release we should do it _before_ we announce 1.10.1 publicly.
> Doing
> > >> it
> > >>>>>>> after 1.10.1 has been announced would only cause more friction
> for
> > >>>>> users.
> > >>>>>>> – Ufuk
> > >>>>>>>
> > >>>>>>> *I hope we don't lose user trust with this. I really would like
> > users
> > >>>>> to be
> > >>>>>>> able to upgrade to the latest patch release without hesitation.
> > >>>>>>>
> > >>>>>>> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <
> > [hidden email]
> > >>>
> > >>>>>>> wrote:
> > >>>>>>>
> > >>>>>>>> Thanks for reporting this issue Seth. This is indeed a big
> > problem.
> > >>>>>>>>
> > >>>>>>>> I looked into the problem and it seems we have the following
> > >>>> situation:
> > >>>>>>>> # 1.9.x --> 1.10.0
> > >>>>>>>>
> > >>>>>>>> There is an API breaking change between 1.9.x and 1.10 due
> > >>>> FLINK-13864
> > >>>>>>>> because it introduced another generic parameter. I expected only
> > few
> > >>>>>>> people
> > >>>>>>>> will be affected by this because one would have to store the
> > builder
> > >>>>> in a
> > >>>>>>>> variable.
> > >>>>>>>>
> > >>>>>>>> 1.9.x is binary compatible with 1.10.0.
> > >>>>>>>>
> > >>>>>>>> # 1.10.0 -> 1.10.1
> > >>>>>>>>
> > >>>>>>>> There is no API breaking change between 1.10.0 and 1.10.1.
> > >>>>>>>>
> > >>>>>>>> I changed the return type of the StreamingFileSink.forRowFormat
> > and
> > >>>>>>>> .forBulkFormat to a subtype of
> StreamingFileSink.BulkFormatBuilder
> > >> in
> > >>>>>>>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the
> > >>>> binary
> > >>>>>>>> incompatibility between 1.10.0 and 1.10.1. This is should not
> > happen
> > >>>>> for
> > >>>>>>>> bug fix releases.
> > >>>>>>>>
> > >>>>>>>> W/o FLINK-16684, the StreamingFileSink builders cannot be used
> > with
> > >>>>>>> Flink's
> > >>>>>>>> Scala API.
> > >>>>>>>>
> > >>>>>>>> # Options
> > >>>>>>>>
> > >>>>>>>> I think we should keep the fix for 1.11.0 and add a release note
> > >> that
> > >>>>> we
> > >>>>>>>> are violating binary compatibility between 1.10 and 1.11.
> > >>>>>>>>
> > >>>>>>>> Now the question is what to do with the 1.10.x branch:
> > >>>>>>>>
> > >>>>>>>> a) We can revert the change to re-establish binary compatibility
> > >>>>> between
> > >>>>>>>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This
> > >>>> would
> > >>>>>>> also
> > >>>>>>>> imply that we cannot use the StreamingFileSink builders with the
> > >>>> Scala
> > >>>>>>> API.
> > >>>>>>>> b) Keep the change and add a release note that our users need to
> > >>>>>>> recompile
> > >>>>>>>> their jobs. This would allow Flink 1.10.x with x >= 1 users to
> use
> > >>>>>>>> StreamingFileSink builders with the Scala API.
> > >>>>>>>>
> > >>>>>>>> I am not entirely sure which need weighs more: binary
> > compatibility
> > >>>>>>> between
> > >>>>>>>> bug fix releases or a working API (small subset of it). Another
> > >>>> aspect
> > >>>>> to
> > >>>>>>>> consider is how many people will migrate from 1.10.0 to 1.10.1
> > >>>> compared
> > >>>>>>> to
> > >>>>>>>> 1.y to 1.10.1 with y <= 9. If the former is very small then one
> > >> might
> > >>>>>>> make
> > >>>>>>>> a case for keeping the change.
> > >>>>>>>>
> > >>>>>>>> What do the others think?
> > >>>>>>>>
> > >>>>>>>> https://issues.apache.org/jira/browse/FLINK-13864
> > >>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
> > >>>>>>>>
> > >>>>>>>> Cheers,
> > >>>>>>>> Till
> > >>>>>>>>
> > >>>>>>>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]>
> > >> wrote:
> > >>>>>>>>
> > >>>>>>>>> We also noticed that and had to make an adjustment downstream.
> > >>>>>>>>>
> > >>>>>>>>> It would be good to mention this in the release notes (if
> that's
> > >> not
> > >>>>>>>>> already the case).
> > >>>>>>>>>
> > >>>>>>>>> Thomas
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <
> > [hidden email]
> > >>>
> > >>>>>>>> wrote:
> > >>>>>>>>>> Hi Everyone,
> > >>>>>>>>>>
> > >>>>>>>>>> I realize I'm about 7 hours late but I just realized there is
> a
> > >>>>>>>> breaking
> > >>>>>>>>>> API change in 1.10.1. FLINK-16684 changes the API of the
> > streaming
> > >>>>>>> file
> > >>>>>>>>>> sink in a binary incompatible way. Since the release has been
> > >>>>>>> approved
> > >>>>>>>>> I'm
> > >>>>>>>>>> not sure the correct course of action but I wanted to bring
> this
> > >> to
> > >>>>>>> the
> > >>>>>>>>>> communities attention.
> > >>>>>>>>>>
> > >>>>>>>>>> Seth
> > >>>>>>>>>>
> > >>>>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
> > >>>>>>>>>>
> > >>>>>
> > >>
> > >>
> >
> >
>

[Till Rohrmann]

The discuss thread for tightening the stability guarantees for
@PublicEvolving classes can be found here [1]

[1]
https://lists.apache.org/thread.html/rb0d0f887b291a490ed3773352c90ddf5f11e3d882dc501e3b8cf0ed0%40%3Cdev.flink.apache.org%3E

Cheers,
Till

On Thu, May 14, 2020 at 10:13 AM Till Rohrmann <[hidden email]> wrote:

> I agree with the pragmatic solution.
>
> Concerning the stability guarantees I will start a separate discussion
> thread whether to provide stricter guarantees or not.
>
> Cheers,
> Till
>
> On Wed, May 13, 2020 at 10:13 PM Thomas Weise <[hidden email]> wrote:
>
>> On Wed, May 13, 2020 at 1:02 PM Piotr Nowojski <[hidden email]>
>> wrote:
>>
>> > Hi,
>> >
>> > > For the future, it would be great to eliminate the possibility of
>> > > incompatible changes to user-facing classes for patch releases though.
>> > > Running japicmp as part of CI might be a good option.
>> >
>> > I guess the whole point of PublicEvolving classes is to allow us for the
>> > breaking compatibility changes.
>> >
>> >
>> The question here is whether such changes should occur in *patch*
>> releases.
>> I suspect most users would prefer stronger guarantees.
>>
>> Another example of "breaking" change that affects Beam (though class
>> OptimizerPlanEnvironment isn't covered by annotation and it only affects
>> test code):
>>
>> https://github.com/apache/beam/pull/11683
>>
>> Piotrek
>> >
>> > > On 13 May 2020, at 18:29, Thomas Weise <[hidden email]> wrote:
>> > >
>> > > For release branches, it would be good to not exclude PublicEvolving
>> from
>> > > checking, by default [1].
>> > >
>> > > There may be instances where it is necessary to make exceptions to
>> fix a
>> > > critical bug, but those could be whitelisted.
>> > >
>> > > Similarly, it might be good to blacklist dependency changes for patch
>> > > releases, by default.
>> > >
>> > > [1]
>> > >
>> >
>> https://github.com/apache/flink/blob/e1e7d7f7ecc080c850a264021bf1b20e3d27d373/pom.xml#L1959
>> > >
>> > > On Wed, May 13, 2020 at 9:16 AM Chesnay Schepler <[hidden email]>
>> > wrote:
>> > >
>> > >> We are running japicmp in CI; we just don't check anything that is
>> > >> PublicEvolving.
>> > >>
>> > >> On 13/05/2020 18:04, Thomas Weise wrote:
>> > >>> +1 for the release note addition
>> > >>>
>> > >>> Historically Flink X.Y.0 releases have been prone to issues like
>> this.
>> > >> And
>> > >>> I suspect that many users would prefer to upgrade to X.Y.1 due to
>> other
>> > >>> bugs that need to be shaken out.
>> > >>>
>> > >>> For the future, it would be great to eliminate the possibility of
>> > >>> incompatible changes to user-facing classes for patch releases
>> though.
>> > >>> Running japicmp as part of CI might be a good option.
>> > >>>
>> > >>> Thanks,
>> > >>> Thomas
>> > >>>
>> > >>> On Wed, May 13, 2020 at 7:54 AM Seth Wiesman <[hidden email]>
>> > >> wrote:
>> > >>>
>> > >>>> +1 Ufuk's pragmatic solution to update the release notes with a
>> public
>> > >>>> notice, I have commented on the release notes PR.
>> > >>>>
>> > >>>> https://github.com/apache/flink-web/pull/330
>> > >>>>
>> > >>>> Seth
>> > >>>>
>> > >>>> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <
>> [hidden email]>
>> > >>>> wrote:
>> > >>>>
>> > >>>>> IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0
>> > >>>> announcement:
>> > >>>>> /"Flink 1.0.0 introduces interface stability annotations for API
>> > >> classes
>> > >>>>> and methods. Interfaces defined as //|@Public|//are guaranteed to
>> > >> remain
>> > >>>>> stable across all releases of the 1.x series. The
>> > >>>>> //|@PublicEvolving|//annotation marks API features that may be
>> > subject
>> > >>>>> to change in future versions."/
>> > >>>>>
>> > >>>>> 1) We don't have a process for promoting things to @Public
>> because we
>> > >>>>> have actually never done that. Based on our current rules, it is
>> an
>> > >>>>> addition to the public API (technically even more so than most
>> > FLIPs),
>> > >>>>> and hence should go through a FLIP and vote.
>> > >>>>> Ideally we evaluate existing @PublicEvolving API's for promotion
>> on
>> > >> each
>> > >>>>> release.
>> > >>>>>
>> > >>>>> 2) Yes, I think this is an excellent idea; as Ufuk said users
>> should
>> > >> not
>> > >>>>> be worried about upgrading to the next minor version. Not sure how
>> > easy
>> > >>>>> this is to implement; but it basically means either modifying or
>> > adding
>> > >>>>> new new japicmp execution when forking the release-X.Y branches.
>> > >>>>>
>> > >>>>> On 13/05/2020 15:19, Till Rohrmann wrote:
>> > >>>>>> A small addendum: The project currently enforces only binary
>> > >>>>> compatibility
>> > >>>>>> for classes which are annotated with @Public. The
>> StreamingFileSink
>> > is
>> > >>>>>> annotated with @PublicEvolving.
>> > >>>>>>
>> > >>>>>> I am not sure whether the community properly defined what kind of
>> > >>>>>> stability/compatibility guarantees we provide for @PublicEvolving
>> > >>>>> classes.
>> > >>>>>> We can derive two follow-up discussions from this:
>> > >>>>>>
>> > >>>>>> 1) Should the StreamingFileSink be made @Public? Should
>> > >>>>>> other @PublicEvolving classes be promoted? What is the process
>> here?
>> > >>>>>>
>> > >>>>>> 2) Should we enforce binary compatibility of @PublicEvolving
>> classes
>> > >>>>>> between bug fix releases and allow breaking changes between
>> > >> minor/major
>> > >>>>>> releases?
>> > >>>>>>
>> > >>>>>> Cheers,
>> > >>>>>> Till
>> > >>>>>>
>> > >>>>>> On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <[hidden email]>
>> wrote:
>> > >>>>>>
>> > >>>>>>> Thanks for the analysis Till.
>> > >>>>>>>
>> > >>>>>>> 1/ I think breaking binary compatibility is acceptable between
>> > minor
>> > >>>>>>> releases (1.10 -> 1.11) as you suggested since the API is marked
>> > >>>>>>> as @PublicEvolving.
>> > >>>>>>>
>> > >>>>>>> 2/ I'm quite torn about how to proceed with the 1.10.x patch
>> > release,
>> > >>>>> but
>> > >>>>>>> slightly leaning towards the "pragmatic" solution of keeping the
>> > >>>> change
>> > >>>>> and
>> > >>>>>>> adding a big warning to the 1.10.1 release announcement*. What
>> do
>> > >>>> others
>> > >>>>>>> think? If we do want to revert the change and follow up with a
>> > 1.10.2
>> > >>>>>>> release we should do it _before_ we announce 1.10.1 publicly.
>> Doing
>> > >> it
>> > >>>>>>> after 1.10.1 has been announced would only cause more friction
>> for
>> > >>>>> users.
>> > >>>>>>> – Ufuk
>> > >>>>>>>
>> > >>>>>>> *I hope we don't lose user trust with this. I really would like
>> > users
>> > >>>>> to be
>> > >>>>>>> able to upgrade to the latest patch release without hesitation.
>> > >>>>>>>
>> > >>>>>>> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <
>> > [hidden email]
>> > >>>
>> > >>>>>>> wrote:
>> > >>>>>>>
>> > >>>>>>>> Thanks for reporting this issue Seth. This is indeed a big
>> > problem.
>> > >>>>>>>>
>> > >>>>>>>> I looked into the problem and it seems we have the following
>> > >>>> situation:
>> > >>>>>>>> # 1.9.x --> 1.10.0
>> > >>>>>>>>
>> > >>>>>>>> There is an API breaking change between 1.9.x and 1.10 due
>> > >>>> FLINK-13864
>> > >>>>>>>> because it introduced another generic parameter. I expected
>> only
>> > few
>> > >>>>>>> people
>> > >>>>>>>> will be affected by this because one would have to store the
>> > builder
>> > >>>>> in a
>> > >>>>>>>> variable.
>> > >>>>>>>>
>> > >>>>>>>> 1.9.x is binary compatible with 1.10.0.
>> > >>>>>>>>
>> > >>>>>>>> # 1.10.0 -> 1.10.1
>> > >>>>>>>>
>> > >>>>>>>> There is no API breaking change between 1.10.0 and 1.10.1.
>> > >>>>>>>>
>> > >>>>>>>> I changed the return type of the StreamingFileSink.forRowFormat
>> > and
>> > >>>>>>>> .forBulkFormat to a subtype of
>> StreamingFileSink.BulkFormatBuilder
>> > >> in
>> > >>>>>>>> FLINK-16684. This causes the java.lang.NoSuchMethodError and
>> the
>> > >>>> binary
>> > >>>>>>>> incompatibility between 1.10.0 and 1.10.1. This is should not
>> > happen
>> > >>>>> for
>> > >>>>>>>> bug fix releases.
>> > >>>>>>>>
>> > >>>>>>>> W/o FLINK-16684, the StreamingFileSink builders cannot be used
>> > with
>> > >>>>>>> Flink's
>> > >>>>>>>> Scala API.
>> > >>>>>>>>
>> > >>>>>>>> # Options
>> > >>>>>>>>
>> > >>>>>>>> I think we should keep the fix for 1.11.0 and add a release
>> note
>> > >> that
>> > >>>>> we
>> > >>>>>>>> are violating binary compatibility between 1.10 and 1.11.
>> > >>>>>>>>
>> > >>>>>>>> Now the question is what to do with the 1.10.x branch:
>> > >>>>>>>>
>> > >>>>>>>> a) We can revert the change to re-establish binary
>> compatibility
>> > >>>>> between
>> > >>>>>>>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2.
>> This
>> > >>>> would
>> > >>>>>>> also
>> > >>>>>>>> imply that we cannot use the StreamingFileSink builders with
>> the
>> > >>>> Scala
>> > >>>>>>> API.
>> > >>>>>>>> b) Keep the change and add a release note that our users need
>> to
>> > >>>>>>> recompile
>> > >>>>>>>> their jobs. This would allow Flink 1.10.x with x >= 1 users to
>> use
>> > >>>>>>>> StreamingFileSink builders with the Scala API.
>> > >>>>>>>>
>> > >>>>>>>> I am not entirely sure which need weighs more: binary
>> > compatibility
>> > >>>>>>> between
>> > >>>>>>>> bug fix releases or a working API (small subset of it). Another
>> > >>>> aspect
>> > >>>>> to
>> > >>>>>>>> consider is how many people will migrate from 1.10.0 to 1.10.1
>> > >>>> compared
>> > >>>>>>> to
>> > >>>>>>>> 1.y to 1.10.1 with y <= 9. If the former is very small then one
>> > >> might
>> > >>>>>>> make
>> > >>>>>>>> a case for keeping the change.
>> > >>>>>>>>
>> > >>>>>>>> What do the others think?
>> > >>>>>>>>
>> > >>>>>>>> https://issues.apache.org/jira/browse/FLINK-13864
>> > >>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
>> > >>>>>>>>
>> > >>>>>>>> Cheers,
>> > >>>>>>>> Till
>> > >>>>>>>>
>> > >>>>>>>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <[hidden email]>
>> > >> wrote:
>> > >>>>>>>>
>> > >>>>>>>>> We also noticed that and had to make an adjustment downstream.
>> > >>>>>>>>>
>> > >>>>>>>>> It would be good to mention this in the release notes (if
>> that's
>> > >> not
>> > >>>>>>>>> already the case).
>> > >>>>>>>>>
>> > >>>>>>>>> Thomas
>> > >>>>>>>>>
>> > >>>>>>>>>
>> > >>>>>>>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <
>> > [hidden email]
>> > >>>
>> > >>>>>>>> wrote:
>> > >>>>>>>>>> Hi Everyone,
>> > >>>>>>>>>>
>> > >>>>>>>>>> I realize I'm about 7 hours late but I just realized there
>> is a
>> > >>>>>>>> breaking
>> > >>>>>>>>>> API change in 1.10.1. FLINK-16684 changes the API of the
>> > streaming
>> > >>>>>>> file
>> > >>>>>>>>>> sink in a binary incompatible way. Since the release has been
>> > >>>>>>> approved
>> > >>>>>>>>> I'm
>> > >>>>>>>>>> not sure the correct course of action but I wanted to bring
>> this
>> > >> to
>> > >>>>>>> the
>> > >>>>>>>>>> communities attention.
>> > >>>>>>>>>>
>> > >>>>>>>>>> Seth
>> > >>>>>>>>>>
>> > >>>>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684
>> > >>>>>>>>>>
>> > >>>>>
>> > >>
>> > >>
>> >
>> >
>>
>