Hi Becket, Thanks for the clarification. Sorry that I didn't make myself clear enough. Let me share more thoughts.
> 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. > Making old experimental API @deprecated with new API as replacement needs extra effort. Let's use the example in FLIP to see what happened. Deprecation rule A: - API foo was introduced in 1.18.x - API foo was carried over in 1.19.0 - API foo was marked as @deprecated in 1.19.1 - API foo is moved in 1.19.2 - the breaking change Deprecation rule B: - API foo was introduced in 1.18.x - API foo was carried over in 1.19.0 - API foo was moved in 1.19.1 - the breaking change - Another release 1.19.2 1.19.2 is the current/last release. One key concern of users is to limit the effort as much as possible for any patch upgrade. Let's name it "the rule of patch upgrade effort". If this is not true, I'd like to know more different thoughts. There will be following scenarios: Scenarios 1: Following the rule of patch upgrade effort: 1. upgrade from 1.18.x to 1.19.x. This is an upgrade across minor releases. Users will upgrade to 1.19.2(the last release), since there will be many bigger changes than the foo-bar change. There is no difference between deprecation rule A and B. 2. upgrade from 1.19.0 to 1.19.x. To minimize the upgrade effort, with the deprecation rule A, users will upgrade to 1.19.1 while with the deprecation rule B, users will stay with 1.19.0 and keep waiting for the next minor release to handle breaking changes with effort. Since the last release is 1.19.2, users are using an old release in both cases. The deprecation rule A is only slightly better, when there are other bug-fixes in 1.19.1 that are urgently required for them. In this case, we will go to the second scenario. Scenarios 2: Don't follow the rule of patch upgrade effort: 1. upgrade from 1.18.x to 1.19.x. Same as in scenario 1. This is an upgrade across minor releases. Users will upgrade to 1.19.2(the last release). There is no difference between deprecation rule A and B. 2. upgrade from 1.19.0 to 1.19.x. Focus will be on upgrading to the last version instead of effort. With both the deprecation rule A and B, users will handle the breaking change and upgrade to 1.19.2. There is no difference between them. As we can see, with the (big) effort of maintaining old experimental APIs, the value is very limited. And in scenario 1.2, since the new patch release 1.19.1 contains urgent bug-fixes, it is acceptable for those users who really need them to put some effort into upgrading (Deprecation rule B). After all, everyone was aware that @experimental can be changed between patch releases in the first place. 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. > I have a different thought. My gut feeling is that, once this FLIP is done, developers will use it as the golden rule each time when deprecated APIs need to be removed. For complex features, the deprecation phase will take time, Marking old APIs deprecated as early as possible will give users more time to migrate or give them the heads-up earlier. It is more comfortable for users than having everything read and marking old APIs as deprecated sharply and removing them after a defined migration period. Because the longer we let such abandoned APIs not be marked as deprecated, the more dependencies users will be building with them, and the bigger effort for users to do migration in the future. > 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. > > Yes, by "backwards compatible", I meant functionally equivalent. You can find more details in this thread[1]. I should have used "functionally compatible" instead. Sorry for the confusion. This is also the example I want to point out for the thought described above. If the FLIP does not contain enough information, guidelines, conditions, the migration period will be the only rule to decide when to remove deprecated APIs without caring of anything else like functionality compatibility. If the migration period described in this FLIP is only the minimum time, I think we still have the house cleaning issue unsolved. Minimum means deprecated APIs can not be removed before the migration period expires. The issue I was aware of is when/how to remove APIs after the migration period expired, e.g. PublicEvolving APIs that have been marked as deprecated years ago. Best regards, Jing [1] https://lists.apache.org/thread/m3o48c2d8j9g5t9s89hqs6qvr924s71o On Wed, Jun 14, 2023 at 9:56 AM 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. > > > > > >
