[GitHub] tzulitai opened a new pull request #7521: [FLINK-11372] Fix incorrect delegation of compatibility checks to new CompositeTypeSerializerSnapshots

[GitBox <gi...@apache.org>]

tzulitai opened a new pull request #7521: [FLINK-11372] Fix incorrect delegation of compatibility checks to new CompositeTypeSerializerSnapshots
URL: https://github.com/apache/flink/pull/7521
 
 
   ## What is the purpose of the change
   
   For deprecated snapshots that are no longer in use (but till kept around for read backwards compatibility), we delegate the compatibility check to its newer replacements.
   For example, the now deprecated `CollectionTypeSerializerConfigSnapshot` delegates compatibility check to the new `ListSerializerSnapshot` if the new serializer to be checked is a `ListSerializer`.
   
   Concretely, the delegation occurs by instantiating an instance of the new snapshot, let it wrap information that was read by the deprecated snapshot class, and let compatibility checks go through the new snapshot's `resolveSchemaCompatibility` method.
   
   There are a few places where the delegation is actually incorrect, because the instantiated new snapshot wrapped information from the new serializer, instead of wrapping information in the legacy snapshot.
   
   ## Brief change log
   
   - 54ab497: A preliminary fix to the test files of `CompositeTypeSerializerMigrationTest`. Specifically, the test files generated for the `EitherSerializer` were using incorrect left / right serializers. This problem only surfaced after applying the fix in this PR.
   - 9b32f0c: Main gist is that a utility method `CompositeTypeSerializerUtil.delegateCompatibilityCheckToNewSnapshot ` is introduced, which handles the delegation. The caller (in legacy snapshots) provides the new serializer to check compatibility for, an instance of the new snapshot to delegate the check to and which already contains outer snapshot information read by the legacy snapshot, as well as an array of nested snapshots read by the legacy snapshot. The utility method builds a valid `CompositeTypeSerializerSnapshot` which is capable of being delegated the compatibility check for the new serializer.
   
   ## Verifying this change
   
   All subclasses of `TypeSerializerSnapshotMigrationTestBase` should still be passing.
   
   ## 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: yes
     - The runtime per-record code paths (performance sensitive): no
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: yes
     - 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
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services