[FLIP-321] Update the docs to add migration periods for deprecated APIs.

[becketqin]

What is the purpose of the change

This PR update the ops doc to add the migration periods for deprecated APIs, which are specified in FLIP-321.

Brief change log

See the title.

Verifying this change

Build the docs and check the website locally at the following address:
http://localhost:1313/docs/ops/upgrading/#deprecated-api-migration-period

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): (no)
• The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
• The serializers: (no)
• The runtime per-record code paths (performance sensitive): (no)
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
• The S3 file system connector: (no)

Documentation

• Does this pull request introduce a new feature? (no)
• If yes, how is the feature documented? (not applicable)

[becketqin]

@mbalassi Do you have time to take a look at the doc update? Thanks!

[flinkbot]

CI report:

• 1e7f692 UNKNOWN
• d03948d Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[JingGe]

Thanks @becketqin for driving this! I just left a tiny comment and look forward to your thoughts.

[JingGe]

Although the source compatibility described in API compatibility guarantees implicitly said that a Public API can only be removed after one major release, it would be easier for us to follow the deprecation rule described in this section(i.e. don't make readers think), if we could explicitly point out that, in this case, for the Public API deprecated in 1.20 and carried over to 2.0, even after 2 minor releases, i.e. 2.0 and 2.1, it still can not be removed in 2.2 and can only be removed in 3.0 at the earliest. WDYT?

[pvary]

@becketqin: We might want to edit the previous section too.

I think it is a bit misleading when we state there that a PublicEvolving API is not source/binary compatible across minor releases. (here we refer to the API as the active version of the API disregarding the deprecation period)

If I understand correctly, this is only true for the active version of the API. Contrary to the table in the docs - because of the proposed deprecation period - the source compatibility will be kept.

Also when we state this:

Code written against a PublicEvolving API in 1.15.2 will continue to run in 1.15.3, without having to recompile the code.
That same code would have to be recompiled when upgrading to 1.16.0 though.

This is only true because of the deprecation period we define later (here we refer to the API with the deprecation period in mind)

[mbalassi]

Thanks for the quick turnaround, @becketqin. I agree with @pvary that we should adjust the table just above the newly added content. For example the Public interface not being binary compatible between minors seems off to me. I asked @gaborgsomogyi to verify as he is more familiar with the japicmp enforcement configuartion that we rely on.

[gaborgsomogyi]

I've had an in-depth look on the compatibility guarantee matrix together with the deprecation policy and here are my findings:

• The doc basically looks good from content perspective though I think adding it is not enough. Please see my later bullet points.
• The binary compatibility matrix has a bug. Namely in Minor release we can't provide source compatibility without binary compatibility (according to the actual source code) because when we do compatibility check then we do both all the time. I think this must be guarantee provided both cases but explaining under what circumstances. (Please see my last bullet point)
• The most important thing is that the binary compatibility matrix is not true anymore. Namely:
  • Some of the API changes are simply not possible because japicmp will block it (good example is minor releases where public API broken). How is planned to deal with japicmp? Personally I can't find any chapter about this.
  • With the new FLIP practically we don't have any kind of hard guarantees from now on which can be expressed with a simple true/false flag. I think the guarantee matrix would be good to be rephrased to reflect the actual state. (Basically all the green marks would be good to be explained under what circumstances are they true). When I would be a user and would see a simple green mark without conditions then I would think that it's true under any circumstances but it's not like that.

[becketqin]

@pvary @mbalassi @gaborgsomogyi @JingGe Thanks a lot for the reviews.

From what I understand, FLIP-321 does not change the API stability guarantee, it only specifies a migration period. For example, PublicEvolving APIs still can be removed with a minor version bump. It is just that with FLIP-321, the removal has to wait for at least an additional minor release.

In terms of the source / binary compatibility, the main reason we do not provide binary compatibility in minor releases for any API stability level is because we treat the protocol / format behind the public API as internal. For example, state format might change although the state backend API remains the same. An RPC protocol may change although the APIs in the user library remain the same. In these cases, users don't have to change their code, but they may have to recompile the code or rerun the job, due to the missing binary compatibility. It is not ideal, but it is what we currently do.

I agree the compatibility table looks confusing, although the contents seem accurate. I am wondering if some more concrete examples can help here.

[gaborgsomogyi]

Thanks for the explanation!

In terms of the source / binary compatibility, the main reason we do not provide binary compatibility in minor releases for any API stability level is because we treat the protocol / format behind the public API as internal.

If I understand correctly then couple of terms/compatibility levels are aggregated in the table. Since we're having japicmp which purely guarantees java based source/binary compatibility I was checking the table only from that point of view. The underneath protocols/internal files compatibility is not considered.
All in all that makes sense. Maybe we can highlight this info in a note to spare some time to others to avoid this pitfall.

I agree the compatibility table looks confusing, although the contents seem accurate. I am wondering if some more concrete examples can help here.

With fresh mind I've re-read/evaluated the whole again and seems like I've thought that Public and PublicEvolving description are having the same pattern in terms of when to remove old API statements (2 and 1 minor release 🙂). I can confirm the migration guide fits with the guarantees matrix.

In general I've the conclusion that existing documentation is valid but users are going to spend quite some time to understand each tiny pieces.

[gaborgsomogyi]

LGTM in general but adding a note to the mentioned red cross would be good.

[pvary]

I would suggest to change the previous paragraph, like the following (remove the API compatibility related parts from there):

The top part could remain the same, just the title changed to Compatibility guarantees:

Compatibility guarantees

The classes & members of the Java/Scala APIs that are intended for users are annotated
with the following stability annotations:

- Public
- PublicEvolving
- Experimental

Annotations on a class also apply to all members of that class, unless otherwise annotated.
Any API without such an annotation is considered internal to Flink, with no guarantees being provided.

Then we should create 2 sub-paragraphs.
The 1st one could be Binary compatibility:

We guarantee binary compatibility only between patch releases for Public, and PublicEvolving interfaces.

Example: Code written against Public and PublicEvolving interfaces in 1.15.2 will continue to run in 1.15.3,
without having to recompile the code.
That same code would have to be recompiled when upgrading to 1.16.0 though, even if no code change is
required based on the API compatibility guarantees.

The 2nd one could be API compatibility, and here could come the paragraph written by @becketqin

[pvary]

Maybe one more column, like:

| Annotation       | Guaranteed Migration Period                   | Could be removed after the migration period |
|:----------------:|:---------------------------------------------:|:-------------------------------------------:|
|     `Public`     | 2 minor releases                              | Next major version                          | 
| `PublicEvolving` | 1 minor release                               | Next minor version                          | 
|  `Experimental`  | 1 patch release for the affected minor release| Next patch version                          | 

[becketqin]

@pvary Sorry for the late response. I read the section a few times and found that the compatibility guarantee statement itself is actually concise and accurate. However, the the example section is a little biased and probably should be articulated a bit more. I updated the patch with a few more examples. Hopefully this makes the documents clear enough.

[pvary]

@becketqin: Thanks for the example changes! They really help to make sense of the previous table.