Hi Xintong, Thanks for the comment. Please see the replies below:
1. Do we allow deprecation & removal of APIs without adding a new one as a > replacement? The examples in the table give me an impression that marking > an API as `@Deprecated` should only happen at the same time of introducing > a new replacing API, which I think is true in most but not all the cases. Right, it is not necessary to have a replacement for the deprecated API, if we decide to sunset the functionality. That does not change the migration period, though. 2. I think this might be a bit too strict. For an API that we already > decided to remove, having to keep it for all the 2.x versions simply > because there's less than 2 minor releases between making the decision and > the major release bump seems not necessarily. Alternatively, I'd like to > propose to remove the `@Public` annotation (or downgrade it to > `@PublicEvolving`) in 2.0, and remove it in 2.2. I am not sure this is a good practice. The purpose of the migration period is to give users enough time to adapt to a breaking API change without holding them back from upgrading Flink. The reason we say Public API needs at least two minor releases is because there are probably more users picked them up over time and more jobs are running using these APIs. So, the public APIs just require a larger migration window. Admittedly, this will introduce a higher maintenance cost for us, this is why Public APIs should be treated seriously. If the promotion of a PublicEvolving API to a Public API requires two minor version releases, deprecation of a Public API should only take longer. Demoting a Public API to PublicEvolving API sounds hacky. From what I understand the stability guarantee is not revocable because users will rely on the stability guarantee to plan their usage of Flink. Demoting an API essentially defeats the very purpose of stability guarantee to begin with. If the concern of keeping a migration period of two minor releases across major versions is about the maintenance overhead, we can choose to bump up the major version to 3.0 at some point after the migration period has passed, assuming by then most of the users have migrated away from the deprecated Public API. Thanks, Jiangjie (Becket) Qin On Wed, Jun 14, 2023 at 4:10 PM Xintong Song <tonysong...@gmail.com> wrote: > Thanks for bringing up this discussion, Becket. > > My two cents: > > 1. Do we allow deprecation & removal of APIs without adding a new one as a > replacement? The examples in the table give me an impression that marking > an API as `@Deprecated` should only happen at the same time of introducing > a new replacing API, which I think is true in most but not all the cases. > > If there is a major version bump before 2 minor releases in the current > > major version are reached, the major version should keep the source code > in > > its own minor version until two minor versions are reached. For example, > in > > the above case, if Flink 2.0 is released after 1.20, then the deprecated > > source code of foo will be kept in 2.0 and all the 2.x versions. It can > > only be removed in 3.0. > > > > 2. I think this might be a bit too strict. For an API that we already > decided to remove, having to keep it for all the 2.x versions simply > because there's less than 2 minor releases between making the decision and > the major release bump seems not necessarily. Alternatively, I'd like to > propose to remove the `@Public` annotation (or downgrade it to > `@PublicEvolving`) in 2.0, and remove it in 2.2. > > Best, > > Xintong > > > > On Wed, Jun 14, 2023 at 3:56 PM Becket Qin <becket....@gmail.com> wrote: > > > Hi Jing, > > > > Thanks for the feedback. Please see the answers to your questions below: > > > > *"Always add a "Since X.X.X" comment to indicate when was a class / > > > interface / method marked as deprecated."* > > > Could you describe it with a code example? Do you mean Java comments? > > > > It is just a comment such as "Since 1.18. Use X > > < > > > https://kafka.apache.org/31/javadoc/org/apache/kafka/clients/admin/Admin.html#incrementalAlterConfigs(java.util.Map) > > >XX > > instead.". And we can then look it up in the deprecated list[1] in each > > release and see which method should / can be deprecated. > > > > *"At least 1 patch release for the affected minor release for > > > Experimental APIs"* > > > The rule is absolutely right. However, afaiac, deprecation is different > > as > > > modification. As a user/dev, I would appreciate, if I do not need to do > > any > > > migration work for any deprecated API between patch releases upgrade. > > BTW, > > > if experimental APIs are allowed to change between patches, could we > just > > > change them instead of marking them as deprecated and create new ones > to > > > replace them? > > > > Deprecating an API is just a more elegant way of replacing an API with a > > new one. The only difference between the two is whether the old API is > kept > > and coexists with the new API for some releases or not. For end users, > > deprecation-then-remove is much more friendly than direct replacement. > > > > 1. How to make sure the new APIs cover all functionality, i.e. backward > > > compatible, before removing the deprecated APIs? Since the > > > functionalities could only be built with the new APIs iteratively, > there > > > will be a while (might be longer than the migration period) that the > new > > > APIs are not backward compatible with the deprecated ones. > > > > This is orthogonal to the deprecation process, and may not even be > required > > in some cases if the function changes by itself. But in general, this > > relies on the developer to decide. A simple test on readiness is to see > if > > all the UT / IT cases written with the old API can be migrated to the new > > one and still work. If the new API is not ready, we probably should not > > deprecate the old one to begin with. > > > > 2. Is it allowed to remove the deprecated APIs after the defined > migration > > > period expires while the new APis are still not backward compatible? > > > > By "backwards compatible", do you mean functionally equivalent? If the > new > > APIs are designed to be not backwards compatible, then removing the > > deprecated source code is definitely allowed. If we don't think the new > API > > is ready to take over the place for the old one, then we should wait. The > > migration period is the minimum time we have to wait before removing the > > source code. A longer migration period is OK. > > > > 3. For the case of core API upgrade with downstream implementations, e.g. > > > connectors, What is the feasible deprecation strategy? Option1 > bottom-up: > > > make sure the downstream implementation is backward compatible before > > > removing the deprecated core APIs. Option2 top-down: once the > downstream > > > implementation of new APIs works fine, we can remove the deprecated > core > > > APIs after the migration period expires. The implementation of the > > > deprecated APIs will not get any further update in upcoming releases(it > > has > > > been removed). There might be some missing features in the downstream > > > implementation of new APIs compared to the old implementation. Both > > options > > > have their own pros and cons. > > > > The downstream projects such as connectors in Flink should also follow > the > > migration path we tell our users. i.e. If there is a cascading backwards > > incompatible change in the connectors due to a backwards incompatible > > change in the core, and as a result a longer migration period is > required, > > then I think we should postpone the removal of source code. But in > general, > > we should be able to provide the same migration period in the connectors > as > > the flink core, if the connectors are upgraded to the latest version of > > core promptly. > > > > Thanks, > > > > Jiangjie (Becket) Qin > > > > [1] > > > > > https://nightlies.apache.org/flink/flink-docs-master/api/java/deprecated-list.html > > > > > > On Wed, Jun 14, 2023 at 1:15 AM Jing Ge <j...@ververica.com.invalid> > > wrote: > > > > > > This is by design. Most of these are @Public APIs that we had to > carry > > > > around until Flink 2.0, because that was the initial guarantee that > we > > > > gave people. > > > > > > > > > > True, I knew @Public APIs could not be removed before the next major > > > release. I meant house cleaning without violation of these annotations' > > > design concept. i.e especially cleaning up for @PublicEvolving APIs > since > > > they are customer-facing. Regular cleaning up with all other > > @Experimental > > > and @Internal APIs would be even better, if there might be some APIs > > marked > > > as @deprecated. > > > > > > Best regards, > > > Jing > > > > > > > > > > > > On Tue, Jun 13, 2023 at 4:25 PM Chesnay Schepler <ches...@apache.org> > > > wrote: > > > > > > > On 13/06/2023 12:50, Jing Ge wrote: > > > > > One major issue we have, afaiu, is caused by the lack of > > > > housekeeping/house > > > > > cleaning, there are many APIs that were marked as deprecated a few > > > years > > > > > ago and still don't get removed. Some APIs should be easy to remove > > and > > > > > others will need some more clear rules, like the issue discussed at > > > [1]. > > > > > > > This is by design. Most of these are @Public APIs that we had to carry > > > > around until Flink 2.0, because that was the initial guarantee that > we > > > > gave people. > > > > > > > > > > > > > > As for the FLIP, I like the idea of explicitly writing down a > > > > deprecation period for APIs, particularly PublicEvolving ones. > > > > For Experimental I don't think it'd be a problem if we could change > > them > > > > right away, > > > > but looking back a bit I don't think it hurts us to also enforce some > > > > deprecation period. > > > > 1 release for both of these sound fine to me. > > > > > > > > > > > > My major point of contention is the removal of Public APIs between > > minor > > > > versions. > > > > This to me would a major setback towards a simpler upgrade path for > > > users. > > > > If these can be removed in minor versions than what even is a major > > > > release? > > > > The very definition we have for Public APIs is that they stick around > > > > until the next major one. > > > > Any rule that theoretically allows for breaking changes in Public API > > in > > > > every minor release is in my opinion not a viable option. > > > > > > > > > > > > The "carry recent Public APIs forward into the next major release" > > thing > > > > seems to presume a linear release history (aka, if 2.0 is released > > after > > > > 1.20, then there will be no 1.21), which I doubt will be the case. > The > > > > idea behind it is good, but I'd say the right conclusion would be to > > not > > > > make that API public if we know a new major release hits in 3 months > > and > > > > is about to modify it. With a regular schedule for major releases > this > > > > wouldn't be difficult to do. > > > > > > > > > >
