You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tillrohrmann <gi...@git.apache.org> on 2017/06/01 15:44:07 UTC
[GitHub] flink pull request #4044: [FLINK-6803] [tests] Add test for PojoSerializer s...
GitHub user tillrohrmann opened a pull request:
https://github.com/apache/flink/pull/4044
[FLINK-6803] [tests] Add test for PojoSerializer state upgrade
The added PojoSerializerUpgradeTest tests the state migration behaviour when the
underlying pojo type changes and one tries to recover from old state. Currently
not all tests could be activated, because there still some pending issues to be
fixed first. We should arm these tests once the issues have been fixed.
cc @tzulitai
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/tillrohrmann/flink addTypeSerializerTests
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/4044.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 #4044
----
commit e2b2fc2ba6ec4a20ff04d47491c5d135e056b8e9
Author: Till Rohrmann <tr...@apache.org>
Date: 2017-05-31T13:14:11Z
[FLINK-6796] [tests] Use Environment's class loader in AbstractStreamOperatorTestHarness
Generalize KeyedOneInputStreamOperatorTestHarness
Generalize AbstractStreamOperatorTestHarness
commit 8ccaba6f6dede0f801608d7fafea5904e81b1624
Author: Till Rohrmann <tr...@apache.org>
Date: 2017-05-31T16:37:12Z
[FLINK-6803] [tests] Add test for PojoSerializer state upgrade
The added PojoSerializerUpgradeTest tests the state migration behaviour when the
underlying pojo type changes and one tries to recover from old state. Currently
not all tests could be activated, because there still some pending issues to be
fixed first. We should arm these tests once the issues have been fixed.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4044: [FLINK-6803] [tests] Add test for PojoSerializer s...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/4044
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4044: [FLINK-6803] [tests] Add test for PojoSerializer s...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/4044#discussion_r120007888
--- Diff: flink-streaming-java/src/test/java/org/apache/flink/streaming/util/AbstractStreamOperatorTestHarness.java ---
@@ -192,7 +187,7 @@ public StreamStatus getStreamStatus() {
when(mockTask.getTaskConfiguration()).thenReturn(underlyingConfig);
when(mockTask.getEnvironment()).thenReturn(environment);
when(mockTask.getExecutionConfig()).thenReturn(executionConfig);
- when(mockTask.getUserCodeClassLoader()).thenReturn(this.getClass().getClassLoader());
+ when(mockTask.getUserCodeClassLoader()).thenReturn(environment.getUserClassLoader());
--- End diff --
Some tests are failing because of this change.
I think the problem is because the given environment may also be a mock whose stubbing isn't completed yet, leading to a `org.mockito.exceptions.misusing.UnfinishedStubbingException`.
We can avoid that by doing this:
```
ClassLoader cl = environment.getUserClassLoader();
when(mockTask.getUserCodeClassLoader()).thenReturn(cl);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #4044: [FLINK-6803] [tests] Add test for PojoSerializer s...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on a diff in the pull request:
https://github.com/apache/flink/pull/4044#discussion_r120008474
--- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/state/StateBackendTestBase.java ---
@@ -1804,53 +1805,44 @@ public void testKeyGroupSnapshotRestore() throws Exception {
}
@Test
- public void testRestoreWithWrongKeySerializer() {
- try {
- CheckpointStreamFactory streamFactory = createStreamFactory();
+ public void testRestoreWithWrongKeySerializer() throws Exception {
+ CheckpointStreamFactory streamFactory = createStreamFactory();
- // use an IntSerializer at first
- AbstractKeyedStateBackend<Integer> backend = createKeyedBackend(IntSerializer.INSTANCE);
+ // use an IntSerializer at first
+ AbstractKeyedStateBackend<Integer> backend = createKeyedBackend(IntSerializer.INSTANCE);
- ValueStateDescriptor<String> kvId = new ValueStateDescriptor<>("id", String.class);
+ ValueStateDescriptor<String> kvId = new ValueStateDescriptor<>("id", String.class);
- ValueState<String> state = backend.getPartitionedState(VoidNamespace.INSTANCE, VoidNamespaceSerializer.INSTANCE, kvId);
+ ValueState<String> state = backend.getPartitionedState(VoidNamespace.INSTANCE, VoidNamespaceSerializer.INSTANCE, kvId);
- // write some state
- backend.setCurrentKey(1);
- state.update("1");
- backend.setCurrentKey(2);
- state.update("2");
+ // write some state
+ backend.setCurrentKey(1);
+ state.update("1");
+ backend.setCurrentKey(2);
+ state.update("2");
- // draw a snapshot
- KeyedStateHandle snapshot1 = FutureUtil.runIfNotDoneAndGet(backend.snapshot(682375462378L, 2, streamFactory, CheckpointOptions.forFullCheckpoint()));
+ // draw a snapshot
+ KeyedStateHandle snapshot1 = FutureUtil.runIfNotDoneAndGet(backend.snapshot(682375462378L, 2, streamFactory, CheckpointOptions.forFullCheckpoint()));
- backend.dispose();
+ backend.dispose();
- // restore with the wrong key serializer
- try {
- restoreKeyedBackend(DoubleSerializer.INSTANCE, snapshot1);
+ // restore with the wrong key serializer
+ try {
+ restoreKeyedBackend(DoubleSerializer.INSTANCE, snapshot1);
- fail("should recognize wrong key serializer");
- } catch (RuntimeException e) {
- if (!e.getMessage().contains("The new key serializer is not compatible")) {
- fail("wrong exception " + e);
- }
- // expected
- }
- }
- catch (Exception e) {
- e.printStackTrace();
- fail(e.getMessage());
+ fail("should recognize wrong key serializer");
+ } catch (StateMigrationException ignored) {
--- End diff --
This change is failing because the `RocksDBKeyedStateBackend` is not throwing the new exception when checking key serializers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4044: [FLINK-6803] [tests] Add test for PojoSerializer state up...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/4044
I'll merge this (will address my own comments) together with the other pending `PojoSerializer` fixes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #4044: [FLINK-6803] [tests] Add test for PojoSerializer state up...
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/4044
Thanks for the review and that you merge the PR @tzulitai.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---