You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tzulitai <gi...@git.apache.org> on 2018/07/02 06:26:32 UTC
[GitHub] flink pull request #6235: [FLINK-9377] [core] Remove serializers from checkp...
GitHub user tzulitai opened a pull request:
https://github.com/apache/flink/pull/6235
[FLINK-9377] [core] Remove serializers from checkpointed state meta infos
## What is the purpose of the change
This PR is the first step towards a smoother state evolution experience.
It removes the behavior of writing serializers in checkpointed state meta infos (using Java serialization) and relying on them to be deserializable at restore time.
Instead, the configuration snapshots of serializers now double as a factory for creating the restore serializer, solidifying it as the single source of truth of information about the previous serializer of state.
With this change:
- Checkpoints / savepoints move towards being Java serialization-free
- The availability of the restore serializer, is basically determined at compile time
- Potentially resolves caveats with macro-generated Scala serializers which typically have anonymous classnames which are easily susceptible to changes, which blocks successful savepoint restores due to how Java serialization works.
- In conclusion: the written configuration snapshot is now the single point of entry for obtaining a serializer for previous state. The user is only required to guarantee that the configuration snapshot's classname remains constant for the restore to proceed.
This PR is only a WIP which only includes extending the `TypeSerializerConfigSnapshot` interface to include a `restoreSerializer` method, as well as the methods interplay in the state backends after removing serializers from checkpointed state meta infos.
This PR does **NOT** include:
- Proper implementation of the new `restoreSerializer` method on all serializers.
- Tests for snapshotting, restoring, and migrating serializers and their interplay in the state backends.
Because of this, it is expected that existing tests will fail.
Follow-up PRs will be opened for the above mentioned missing parts.
## Brief change log
- 5fc4a36 Add a `restoreSerializer` method to the `TypeSerializerConfigSnapshot` interface
The method still has a dummy base implementation, because this PR doesn't yet properly implement the method for all serializers. Once that is accomplished, the base implementation should be removed.
- 661eb6d Remove the "fallback" serializer option from `CompatibilityResult`
That option was available in the past to allow users to have a safety path for state conversion, in case their previous serializer cannot be deserialized due to any reason blocked by Java serialization. Since now we use the config snapshot as the restore serializer factory, it is guaranteed that the restore serializer is always available in case conversion is required, and therefore voids the need for the "fallback" serializer option.
- c91d045 Deprecates any utility methods that still have the behaviour of writing serializers in checkpoints
- e09f914 Introduces the `BackwardsCompatibleConfigSnapshot` class
The BackwardsCompatibleConfigSnapshot is a wrapper, dummy config
snapshot which wraps an actual config snapshot, as well as a
pre-existing serializer instance.
In previous versions, since the config snapshot wasn't a serializer
factory but simply a container for serializer parameters, previous
serializers didn't necessarily have config snapshots that are capable of
correctly creating a correct corresponding restore serializer.
In this case, since previous serializers still have serializers written
in the checkpoint, the backwards compatible solution would be to wrap
the written serializer and the config snapshot within the
BackwardsCompatibleConfigSnapshot dummy. When attempting to restore the
serializer, the wrapped serializer instance is returned instead of
actually calling the restoreSerializer method of the wrapped config
snapshot.
- da84665 the actual removal of serializers from checkpointed state meta info
## Verifying this change
This PR is a WIP preview, and tests is expected to fail due to reasons mentioned in the description.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes / **no**)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (**yes** / no)
- The serializers: (**yes** / no / don't know)
- The runtime per-record code paths (performance sensitive): (yes / **no** / don't know)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (**yes** / no / don't know)
- The S3 file system connector: (yes / **no** / don't know)
## Documentation
- Does this pull request introduce a new feature? (yes / **no**)
- If yes, how is the feature documented? (not applicable / docs / **JavaDocs** / not documented)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tzulitai/flink FLINK-9377
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/6235.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #6235
----
commit 5fc4a36a144c3f8f22be7e21a4e542d3042d10b1
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-06-13T11:43:53Z
[FLINK-9377] [core] (part 1) Extend TypeSerializerConfigSnapshot as a factory for restoring serializers
This commit is the first step towards removing serializers from
checkpointed state meta info and making Flink checkpoints Java
serialization free.
Instead of writing serializers in checkpoints, and trying to read that
to obtain a restore serializer at restore time, we aim to only write the
config snapshot as the single source of truth and use it as a factory to
create a restore serializer.
This commit adds the method and signatures to the
TypeSerializerConfigSnapshot interface. Use of the method, as well as
properly implementing the method for all serializers, will be
implemented in follow-up commits.
commit 661eb6d34da450ed096a77f166a4cc62ce3efdba
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-06-14T09:52:06Z
[FLINK-9377] [core] (part 2) Remove fallback deserializer option from CompatibilityResult
Now that the config snapshot is used as a factory for the restore
serializer, it should be guaranteed that a restore serializer is always
available. This removes the need for the user to provide a "fallback"
convert serializer in the case where a migration is required.
commit c91d045c5eb6e355981e4edaa6d1a0d48e5d4a5e
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-06-14T14:41:45Z
[FLINK-9377] [core] (part 3) Deprecate TypeSerializerSerializationUtil
This commit deprecates all utility methods and classes related to
serializing serializers. All methods that will still be in use, i.e.
writing config snapshots, are now moved to a separate new
TypeSerializerConfigSnapshotSerializationUtil class.
commit e09f91469fb6c86f5d2f05b78a9db3d9af8cce87
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-06-18T14:24:08Z
[FLINK-9377] [core] (part 4) Introduce BackwardsCompatibleConfigSnapshot
The BackwardsCompatibleConfigSnapshot is a wrapper, dummy config
snapshot which wraps an actual config snapshot, as well as a
pre-existing serializer instance.
In previous versions, since the config snapshot wasn't a serializer
factory but simply a container for serializer parameters, previous
serializers didn't necessarily have config snapshots that are capable of
correctly creating a correct corresponding restore serializer.
In this case, since previous serializers still have serializers written
in the checkpoint, the backwards compatible solution would be to wrap
the written serializer and the config snapshot within the
BackwardsCompatibleConfigSnapshot dummy. When attempting to restore the
serializer, the wrapped serializer instance is returned instead of
actually calling the restoreSerializer method of the wrapped config
snapshot.
commit da84665a9b101a803f7446210afc34bbd4a71703
Author: Tzu-Li (Gordon) Tai <tz...@...>
Date: 2018-07-02T03:45:20Z
[FLINK-9377] [core] (part 5) Remove serializers from checkpoint state meta infos
This commit officially removes the behaviour of writing serializers in
the state meta info of keyed state, operator state, and timers state.
This affects the serialization formats of the
KeyedBackendSerializationProxy, OperatorBackendSerializationProxy, and
InternalTimerServiceSerializationProxy, and therefore their versions are
all upticked.
----
---
[GitHub] flink issue #6235: [FLINK-9377] [core] Remove serializers from checkpointed ...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/6235
Took a look at this WIP and I think it goes into a good direction.
My most important comment is that I think it would help to move the "ensureCompatibility" into the config snapshot, for the following reasons:
- Clearer separation of concerns, the serializer has only the serialization logic, and creating the snapshot. Compatibility is not the serializers immediate concern.
- The current design means that the serializer mutates internal fields on reconfiguration. That is error prone. Consider a serializer like the KryoSerializer, where the configuration is not fully deep copied on duplication (makes sense, it is read only during serialization). Mutating that configuration would change the behavior of other previously duplicated serializers as well, which is unexpected.
Thoughts for improvements with lower priority:
- Can we avoid setting the ClassLoader into a field in the config snapshot, and then deserializing? I think such solutions are fragile and should be avoided if possible. The ClassLoader is not really part of the snapshots state, it is an auxiliary to the deserialization and should, as such, be passed as an argument to the read method: read(in, classloader). This means that the TypeSerializerConfigSnapshot would not implement `IOReadableWritable`, but that might be not a problem.
- Is the TypeSerializerConfigSnapshotSerializationProxy needed? It seems like an unnecessary indirection given that it is used exclusively in the TypeSerializerSerializationUtil and could be a static util method instead.
---