You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by guoweiM <gi...@git.apache.org> on 2017/04/05 09:13:31 UTC

[GitHub] flink pull request #3672: [Flink-6072] TestCase CheckpointStateRestoreTest::...

GitHub user guoweiM opened a pull request:

    https://github.com/apache/flink/pull/3672

    [Flink-6072] TestCase CheckpointStateRestoreTest::testSetState should be fail

    This case should be fail because three `KeyGroupStateHandles` which have same `KeyGroupRange[0,0]` are received by `CheckpointCoordinator`.
    
    This pull request includes:
    1. Use three different `KeyGroupRange`s in the test case.
    2. Check whether the `KeyGroupRange` of `TaskState` has overlap.
    3. Check whether the `KeyGroupRange` of `SubTaskState` is different.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/guoweiM/flink FLINK-6072

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/3672.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 #3672
    
----
commit b833bd49b524d0fc1ef75ba2c82651432bdb84c1
Author: guowei.mgw <gu...@gmail.com>
Date:   2017-03-23T23:17:09Z

    fix CheckpointStateRestoreTest

commit a816f61c562e2d713c35b35037dedb444f656683
Author: guowei.mgw <gu...@gmail.com>
Date:   2017-04-05T08:09:20Z

    Fix FLINK-5892

commit c498dea78675a529d032857a00f6fbaf36c0c16e
Author: guowei.mgw <gu...@gmail.com>
Date:   2017-04-05T08:13:44Z

    Fix FLINK-5892

----


---
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 #3672: [Flink-6072] TestCase CheckpointStateRestoreTest::...

Posted by pnowojski <gi...@git.apache.org>.
Github user pnowojski commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3672#discussion_r127641370
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/SubtaskState.java ---
    @@ -20,6 +20,7 @@
     
     import org.apache.flink.runtime.state.ChainedStateHandle;
    --- End diff --
    
    ditto - please add commit message and title


---
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 #3672: [Flink-6072] TestCase CheckpointStateRestoreTest::...

Posted by pnowojski <gi...@git.apache.org>.
Github user pnowojski commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3672#discussion_r127641085
  
    --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/CheckpointStateRestoreTest.java ---
    @@ -66,10 +66,19 @@
     	public void testSetState() {
     		try {
     
    -			final ChainedStateHandle<StreamStateHandle> serializedState = CheckpointCoordinatorTest.generateChainedStateHandle(new SerializableObject());
    -			KeyGroupRange keyGroupRange = KeyGroupRange.of(0,0);
    -			List<SerializableObject> testStates = Collections.singletonList(new SerializableObject());
    -			final KeyedStateHandle serializedKeyGroupStates = CheckpointCoordinatorTest.generateKeyGroupState(keyGroupRange, testStates);
    +			final ChainedStateHandle<StreamStateHandle> serializedState1 = CheckpointCoordinatorTest.generateChainedStateHandle(new SerializableObject());
    --- End diff --
    
    Could you write in commit message what are actually fixing? And could you also add some commit title that sums up your changes?
    
    After this commit, will this test start failing and only next commits fix that?


---
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 #3672: [Flink-6072] TestCase CheckpointStateRestoreTest::...

Posted by pnowojski <gi...@git.apache.org>.
Github user pnowojski commented on a diff in the pull request:

    https://github.com/apache/flink/pull/3672#discussion_r127641519
  
    --- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/runtime/tasks/StreamTask.java ---
    @@ -949,8 +949,6 @@ public void run() {
     						checkpointMetaData.getCheckpointId());
     				}
     			} catch (Exception e) {
    -				e.printStackTrace();;
    --- End diff --
    
    Please squash with previous change or mark this as a fixup (`git ci --amend --fixup=HEAD^`)


---
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.
---