Hi Hangxiang, Dawid,
I also prefer to add method into TypeSerializerSnapshot, which looks
more natural. TypeSerializerSnapshot has `Version` concept, which also
can be used for compatibility.
`
TypeSerializerSnapshot {
TypeSerializerSchemaCompatibility<T> resolveSchemaCompatibility(
TypeSerializerSnapshot<T> oldSnapshot);
}
`
Best,
Godfrey
Dawid Wysakowicz <dwysakow...@apache.org> 于2022年10月20日周四 16:59写道：
>
> 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.
>
>
> I understand it works for old serializers, but it makes writing new
> serializers more complicated. It's not as simple as extending an interface
> and implementing all the methods it tells you to. You have to dig into
> documentation and check that you must implement one of the methods (either
> the old or the new one). Otherwise you end up in an ifinite loop.
>
>
> If we break the compatibility, yes we cause some annoyance, but I feel it's a
> safer option. BTW, there is a proposal around serializer that also causes
> some incompatibilities so we could squash this together:
> https://lists.apache.org/thread/v1q28zg5jhxcqrpq67pyv291nznd3n0w
>
>
> I don't want to force that, but would be really happy to hear what others
> think.
>
>
> Best,
>
> Dawid
>
> On 19/10/2022 04:05, Hangxiang Yu wrote:
>
> 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:
>
> TypeSerializerSnapshot still owns the responsibility of resolving schema
> compatibility so TypeSerializer could just pay attention to its serialization
> and deserialization as before.
> 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:
>
> 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.
