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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to