[FLINK-11287] [rocksdb] RocksDBListState should be using registered serializer in state meta infos#7434
Closed
tzulitai wants to merge 2 commits intoapache:masterfrom
Closed
[FLINK-11287] [rocksdb] RocksDBListState should be using registered serializer in state meta infos#7434tzulitai wants to merge 2 commits intoapache:masterfrom
tzulitai wants to merge 2 commits intoapache:masterfrom
Conversation
…erializer in state meta infos
…tateSerializerReconfiguration test in StateBackendMigrationTestBase This test was previously ignored due to 2 missing changes: - ListSerializerSnapshot was not respecting serializer reconfiguration (fixed by FLINK-11073), and - RocksDBListState did not use the correct registered state serializer in backend's state meta info (fixed by FLINK-11287) With those fixes in, the test can now be activated and expected to pass.
Contributor
|
LGTM 👍 |
Contributor
Author
|
Thanks, merging ... |
tisonkun
pushed a commit
to tisonkun/flink
that referenced
this pull request
Jan 17, 2019
…erializer in state meta infos This closes apache#7434.
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
3 participants
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.
What is the purpose of the change
RocksDBListStatewas using the serializer provided by the state descriptor, which is incorrect because that serializer have not been checked for compatibility or reconfigured if required.Instead, it should be using the actual resulting registered serializer in the backend's state meta infos, which is guaranteed to have been checked for compatibility.
Brief change log
StateBackendMigrationTestBase. That test wasn't passing due to FLINK-11073 and FLINK-11287 (which this PR fixes).Verifying this change
The previously ignored test
StateBackendMigrationTestBase.testKeyedListStateSerializerReconfigurationshould now pass, as FLINK-11287 is partially the reason of why it wasn't passing before.That test verifies that
RocksDBListStateis indeed using the correct, compatibility-checked / reconfigured serializer.Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation