Hi, Dawid.

Thanks for your reply.

Should we introduce the new method to the TypeSerializerSnapshot instead?

You provided a valuable option, let me try to list the benefit and cost.

benefit:

   1. TypeSerializerSnapshot still owns the responsibility of resolving
   schema compatibility so TypeSerializer could just pay attention to its
   serialization and deserialization as before.
   2. It's very convenient to implement it based on current implementation
   by all information in TypeSerializerSnapshot and tools which is also
   helpful for users to migrate their external serializer.

cost:

   1. The interface is a bit strange because we have to construct and use
   the snapshot of the new serializer to check the compatibility.

I appreciate this because users could migrate more conveniently.

For its cost, We could see TypeSerializerSnapshot as a point-in-time view
of TypeSerializer used for forward compatibility checks and persisting
serializer within checkpoints. It seems reasonable to generate a
point-in-time view of the serializer when we need to resolve compatibility.


I'd actually be fine with breaking some external serializers and not provide
a default implementation of the new method.

That's the final status we will arrive at.

IIUC, we cannot just remove the original method but just mark it as
deprecated so two methods have to exist together in the short term.

Users may also need to migrate their own external serializers which is a
long run.

I'd like to make jobs where users could always work without modifying any
codes before removing the deprecated method so I provide a default
implementation in the proposal.

On Mon, Oct 17, 2022 at 10:10 PM Dawid Wysakowicz <dwysakow...@apache.org>
wrote:

> Hi Han,
>
> I think in principle your proposal makes sense and the compatibility
> check indeed should be done in the opposite direction.
>
> However, I have two suggestions:
>
> 1. Should we introduce the new method to the TypeSerializerSnapshot
> instead? E.g.
>
>     TypeSerializerSnapshot {
>
>         TypeSerializerSchemaCompatibility<T> resolveSchemaCompatibility(
>              TypeSerializerSnapshot<T> oldSnapshot);
>
>     }
>
> I know it has the downside that we'd need to create a snapshot for the
> new serializer during a restore, but the there is a lot of tooling and
> subclasses in the *Snapshot stack that it won't be possible to migrate
> to TypeSerializer stack. I have classes such as
> CompositeTypeSerializerSnapshot and alike in mind.
>
> 2. I'd actually be fine with breaking some external serializers and not
> provide a default implementation of the new method. If we don't do that
> we will have two methods with default implementation which call each
> other. It makes it also a bit harder to figure out which methods are
> mandatory to be implemented going forward.
>
> What do you think?
>
> Best,
>
> Dawid
>
> On 17/10/2022 04:43, Hangxiang Yu wrote:
> > Hi, Han.
> >
> > Both the old method and the new method can get previous and new inner
> > information.
> >
> > The new serializer will decide it just like the old serializer did
> before.
> >
> > The method just specify the schema compatibility result so that other
> > behaviours is same as before.
> >
> > On Fri, Oct 14, 2022 at 11:40 AM Han Yin <alexyin...@gmail.com> wrote:
> >
> >> Hi Hangxiang,
> >>
> >> Thanks for the proposal. It seems more reasonable to let the new
> >> serializer claim the compatibility in the cases you mentioned.
> >>
> >> I have but one question here. What happens in the case of
> >> “compatibleAfterMigration” after we completely reverse the direction (in
> >> step 3)?  To be specific, migration from an old schema calls for the
> >> previous serializer to read bytes into state objects. How should a new
> >> serializer decide whether the migration is possible?
> >>
> >> Best,
> >> Han
> >>
> >> On 2022/10/12 12:41:07 Hangxiang Yu wrote:
> >>> Dear Flink developers,
> >>>
> >>> I would like to start a discussion thread on FLIP-263[1] proposing to
> >>> improve the usability of resolving schema compatibility.
> >>>
> >>> Currently, the place for compatibility checks is
> >>> TypeSerializerSnapshot#resolveSchemaCompatibility
> >>> which belongs to the old serializer, There are no ways for users to
> >> specify the
> >>> compatibility with the old serializer in the new customized serializer.
> >>>
> >>> The FLIP hopes to reverse the direction of resolving schema
> compatibility
> >>> to improve the usability of resolving schema compatibility.
> >>>
> >>> [1]
> >>>
> >>
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-263%3A+Improve+resolving+schema+compatibility
> >
> >
>


-- 
Best,
Hangxiang.

Reply via email to