[FLINK-40297][runtime] Route TTL-aware value migration through the migrate hook - #28881
Draft
weiqingy wants to merge 2 commits into
Draft
[FLINK-40297][runtime] Route TTL-aware value migration through the migrate hook#28881weiqingy wants to merge 2 commits into
weiqingy wants to merge 2 commits into
Conversation
…apshot
Add a default method that lets a serializer snapshot transform an already
deserialized state value from the schema it was written with into the schema
the current serializer expects:
default T migrate(TypeSerializerSnapshot<T> oldSerializerSnapshot, T value)
Like resolveSchemaCompatibility, it is invoked on the new snapshot and receives
the old snapshot as its argument. The default returns the value unchanged, so
behavior is unaffected for every existing serializer: a value deserialized with
the prior serializer is structurally compatible with the current one and can be
re-serialized as is.
The javadoc states that migration is not applied recursively to nested
serializers. Unlike resolveSchemaCompatibility, which CompositeTypeSerializer-
Snapshot delegates to the nested snapshots, migrate has no delegating override,
so a composite returns its value unmigrated unless it decomposes the value
itself. That asymmetry is invisible at the call site and would otherwise fail
silently.
Generated-by: Claude Code (Opus 5)
…grate hook TtlAwareSerializer.migrateValueFromPriorSerializer is the single entry point through which the RocksDB state backend migrates state values on restore: AbstractRocksDBState, RocksDBListState and RocksDBMapState all call it after unwrapping the state shape they own. It deserialized with the prior serializer and re-serialized with the new one, leaving a serializer no opportunity to adapt the value in between. Route it through TypeSerializerSnapshot.migrate: unwrap the prior value to its bare, non-TTL form, migrate it, then re-wrap when this serializer is TTL-enabled, preserving the prior timestamp when the prior value carried one. Behavior is unchanged, because no serializer overrides the hook yet and its default returns the value unchanged. The hook receives the persisted prior snapshot rather than one re-derived by calling snapshotConfiguration() on the restored prior serializer. That round trip is lossy: PojoSerializerSnapshot substitutes a synthetic name for a field that no longer exists on the class, so a migrate override reconciling fields by name would see a fabricated schema. The backend already holds the persisted snapshot above the migration loop, and each caller now descends it alongside its serializer. The descent unwraps the TtlAware decorator first. Registering a new serializer mutates the previous snapshot's nested snapshots in place, so a list or map state's persisted element or value snapshot is a TtlAwareSerializerSnapshot rather than the snapshot the checkpoint wrote. A snapshot of an unexpected type now fails rather than falling back to a re-derived one; only an absent snapshot falls back. Generated-by: Claude Code (Opus 5)
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the second PR of the FLIP-527 implementation, split into a stack of small, independently reviewable PRs under the umbrella issue FLINK-37732. Landing order:
migratehook onTypeSerializerSnapshotRowDataDraft until PR-1 merges: this branch contains PR-1's commit, so the diff against master shows both. It will be rebased onto master and marked ready once PR-1 lands. PR-1 and PR-2 are behavior-neutral, since the hook defaults to returning its argument and nothing overrides it until PR-3.
What is the purpose of the change
TtlAwareSerializer.migrateValueFromPriorSerializeris the single entry point through which the RocksDB state backend migrates state values on restore.AbstractRocksDBState,RocksDBListStateandRocksDBMapStateall call it, each after unwrapping the state shape it owns: the value serializer, the list element serializer, or the map value serializer.Today it deserializes with the prior serializer and re-serializes with the new one, giving a serializer no opportunity to adapt the value in between. This routes it through the
migratehook added in PR-1, so a serializer that overrides the hook takes effect. Behavior is unchanged, because nothing overrides it yet and the default returns the value unchanged.Two details are worth calling out for review, because both are contract decisions rather than mechanics.
The hook receives the persisted prior snapshot, not a re-derived one. The prior serializer reaching this method is itself
previousSerializerSnapshot.restoreSerializer(), so callingsnapshotConfiguration()on it is a snapshot to serializer to snapshot round trip. That round trip is lossy:PojoSerializerSnapshotinserts anullFieldfor a field that no longer exists on the class, and re-snapshotting substitutes a synthetic name for it. An override reconciling fields by name, which is what FLIP-527 adds in PR-3, would then see a fabricated schema. The backend already fetches the persisted snapshot above the migration loop, so this threads it down and each caller descends it alongside the serializer it already descends.The descent unwraps the TtlAware decorator first, and an unexpected snapshot type now fails rather than falling back. Registering a new serializer mutates the previous snapshot's nested snapshots in place, so a list or map state's persisted element or value snapshot is a
TtlAwareSerializerSnapshotrather than the snapshot the checkpoint wrote. An earlier revision of this change assumed otherwise and fell back silently to the re-derived snapshot on exactly those paths; the assertion added here is what surfaced it. Only an absent snapshot falls back now, which is the case where nothing was persisted and the re-derived snapshot is all that exists.Brief change log
migrate, and re-wrap when this serializer is TTL-enabled, preserving the prior TTL timestamp when the prior value carried onevalueparameter, and note that old and new snapshots need not expose the same nested snapshot typesVerifying this change
This change added tests and can be verified as follows:
TtlAwareSerializerTestcovers all four combinations of prior and current TTL being enabled, asserting migrated bytes and the preserved timestamp. These were written to pass against the pre-change code, so they demonstrate that behavior is unchanged rather than asserting itStateSerializerProviderTestpins the in-place mutation of nested snapshots that the descent depends on, so a change to that mechanism fails here rather than silently downstreamDoes this pull request potentially affect one of the following parts:
@Public(Evolving): no.TypeSerializerSnapshotis@PublicEvolvingbut only its javadoc changed. Three internal backend method signatures widened; none is on an interface or carries a stability annotation, and japicmp is green.Documentation
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5)