You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/04/14 03:28:49 UTC

[GitHub] [flink] Myasuka opened a new pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Myasuka opened a new pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721
 
 
   ## What is the purpose of the change
   
   FLINK-14652 planed to focus on refactoring checkpoint related code into one place on task side  The `SubtaskCheckpointCoordinator` which introduced recently in FLINK-16744 actually already implement most of this work. This PR would introduce `notifyCheckpointComplete` to `SubtaskCheckpointCoordinator` to let that coordinator could coordinate more work on task side before we introduce `notifyCheckpointAborted`. 
   
   ## Brief change log
   
   Introduce `notifyCheckpointComplete` to `SubtaskCheckpointCoordinator`.
   
   ## Verifying this change
   This change added tests and can be verified as follows:
   
     - `SubtaskCheckpointCoordinatorTest#testNotifyCheckpointComplete` would verify the logic.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
     - The serializers: no
     - The runtime per-record code paths (performance sensitive): no
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
     - The S3 file system connector: no
   
   ## Documentation
   
     - Does this pull request introduce a new feature? no
     - If yes, how is the feature documented? not applicable
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] Myasuka commented on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
Myasuka commented on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613269514
 
 
   I think `SubtaskCheckpointCoordinator` could play the role to contain all checkpoint related action on task side, and I use `SubtaskCheckpointCoordinator` to place `notifyCheckpointComplete`. 
   @rkhachatryan , would you please take a look at this PR?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613210240
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160148053",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6df99687837974586fd4d9892d0fd0bab05374bc Travis: [PENDING](https://travis-ci.com/github/flink-ci/flink/builds/160148053) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot commented on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613210240
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6df99687837974586fd4d9892d0fd0bab05374bc UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613210240
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160148053",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6df99687837974586fd4d9892d0fd0bab05374bc Travis: [SUCCESS](https://travis-ci.com/github/flink-ci/flink/builds/160148053) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613210240
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160148053",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7511",
       "triggerID" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "triggerType" : "PUSH"
     }, {
       "hash" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160362281",
       "triggerID" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 84ffc6cd70dd762d314fcbb5d09e975de8e44603 Travis: [SUCCESS](https://travis-ci.com/github/flink-ci/flink/builds/160362281) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7511) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] rkhachatryan commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#discussion_r408366858
 
 

 ##########
 File path: flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/SubtaskCheckpointCoordinatorTest.java
 ##########
 @@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.streaming.runtime.tasks;
+
+import org.apache.flink.runtime.operators.testutils.MockEnvironment;
+import org.apache.flink.runtime.state.TestTaskStateManager;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+
+/**
+ * Tests for {@link SubtaskCheckpointCoordinator}.
+ */
+public class SubtaskCheckpointCoordinatorTest {
+
+	@Test
+	public void testNotifyCheckpointComplete() throws Exception {
+		TestTaskStateManager stateManager = new TestTaskStateManager();
+		MockEnvironment mockEnvironment = MockEnvironment.builder().setTaskStateManager(stateManager).build();
+		SubtaskCheckpointCoordinator subtaskCheckpointCoordinator = new MockSubtaskCheckpointCoordinatorBuilder()
+			.setEnvironment(mockEnvironment)
+			.build();
+
+		long checkpointId = 42L;
+		{
+			boolean success = subtaskCheckpointCoordinator.notifyCheckpointComplete(checkpointId, mock(OperatorChain.class), () -> true);
 
 Review comment:
   I think this (mockito mock) can break easily once some more complex behavior is be expected from the `OperatorChain` (more [here](https://flink.apache.org/contributing/code-style-and-quality-common.html#design-for-testability)).
   Instead, this should work:
   ```
   final OperatorChain chain = new OperatorChain<>(
     new MockStreamTaskBuilder(new DummyEnvironment()).build(),
     new NonRecordWriter<>());
   boolean success = subtaskCheckpointCoordinator.notifyCheckpointComplete(newCheckpointId, chain, () -> false);
   
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot commented on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
flinkbot commented on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613207155
 
 
   Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
   to review your pull request. We will use this comment to track the progress of the review.
   
   
   ## Automated Checks
   Last check on commit 6df99687837974586fd4d9892d0fd0bab05374bc (Tue Apr 14 03:31:05 UTC 2020)
   
   **Warnings:**
    * No documentation files were touched! Remember to keep the Flink docs up to date!
   
   
   <sub>Mention the bot in a comment to re-run the automated checks.</sub>
   ## Review Progress
   
   * ❓ 1. The [description] looks good.
   * ❓ 2. There is [consensus] that the contribution should go into to Flink.
   * ❓ 3. Needs [attention] from.
   * ❓ 4. The change fits into the overall [architecture].
   * ❓ 5. Overall code [quality] is good.
   
   Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
    The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
    - `@flinkbot approve all` to approve all aspects
    - `@flinkbot approve-until architecture` to approve everything until `architecture`
    - `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
    - `@flinkbot disapprove architecture` to remove an approval you gave earlier
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613210240
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160148053",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "status" : "PENDING",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7511",
       "triggerID" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "triggerType" : "PUSH"
     }, {
       "hash" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "status" : "PENDING",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160362281",
       "triggerID" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6df99687837974586fd4d9892d0fd0bab05374bc Travis: [SUCCESS](https://travis-ci.com/github/flink-ci/flink/builds/160148053) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421) 
   * 84ffc6cd70dd762d314fcbb5d09e975de8e44603 Travis: [PENDING](https://travis-ci.com/github/flink-ci/flink/builds/160362281) Azure: [PENDING](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7511) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613210240
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "DELETED",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "DELETED",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160148053",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7511",
       "triggerID" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "triggerType" : "PUSH"
     }, {
       "hash" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160362281",
       "triggerID" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 84ffc6cd70dd762d314fcbb5d09e975de8e44603 Travis: [SUCCESS](https://travis-ci.com/github/flink-ci/flink/builds/160362281) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7511) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] Myasuka commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
Myasuka commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#discussion_r408684355
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/state/TestTaskStateManager.java
 ##########
 @@ -46,6 +46,7 @@
 public class TestTaskStateManager implements TaskStateManager {
 
 	private long reportedCheckpointId;
+	private long notifiedCompletedCheckpointId;
 
 Review comment:
   I think just retaining the last notified completed checkpoint id should be enough like previous `reportedCheckpointId` did.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613210240
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160148053",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6df99687837974586fd4d9892d0fd0bab05374bc Travis: [SUCCESS](https://travis-ci.com/github/flink-ci/flink/builds/160148053) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421) 
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on issue #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#issuecomment-613210240
 
 
   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "SUCCESS",
       "url" : "https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "status" : "SUCCESS",
       "url" : "https://travis-ci.com/github/flink-ci/flink/builds/160148053",
       "triggerID" : "6df99687837974586fd4d9892d0fd0bab05374bc",
       "triggerType" : "PUSH"
     }, {
       "hash" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "84ffc6cd70dd762d314fcbb5d09e975de8e44603",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 6df99687837974586fd4d9892d0fd0bab05374bc Travis: [SUCCESS](https://travis-ci.com/github/flink-ci/flink/builds/160148053) Azure: [SUCCESS](https://dev.azure.com/rmetzger/5bd3ef0a-4359-41af-abca-811b04098d2e/_build/results?buildId=7421) 
   * 84ffc6cd70dd762d314fcbb5d09e975de8e44603 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run travis` re-run the last Travis build
    - `@flinkbot run azure` re-run the last Azure build
   </details>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] rkhachatryan commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#discussion_r408366858
 
 

 ##########
 File path: flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/SubtaskCheckpointCoordinatorTest.java
 ##########
 @@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.streaming.runtime.tasks;
+
+import org.apache.flink.runtime.operators.testutils.MockEnvironment;
+import org.apache.flink.runtime.state.TestTaskStateManager;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+
+/**
+ * Tests for {@link SubtaskCheckpointCoordinator}.
+ */
+public class SubtaskCheckpointCoordinatorTest {
+
+	@Test
+	public void testNotifyCheckpointComplete() throws Exception {
+		TestTaskStateManager stateManager = new TestTaskStateManager();
+		MockEnvironment mockEnvironment = MockEnvironment.builder().setTaskStateManager(stateManager).build();
+		SubtaskCheckpointCoordinator subtaskCheckpointCoordinator = new MockSubtaskCheckpointCoordinatorBuilder()
+			.setEnvironment(mockEnvironment)
+			.build();
+
+		long checkpointId = 42L;
+		{
+			boolean success = subtaskCheckpointCoordinator.notifyCheckpointComplete(checkpointId, mock(OperatorChain.class), () -> true);
 
 Review comment:
   I think this (mockito mock) can break easily once some more complex behavior is be expected from the `OperatorChain`.
   Instead, this should work:
   ```
   final OperatorChain chain = new OperatorChain<>(
     new MockStreamTaskBuilder(new DummyEnvironment()).build(),
     new NonRecordWriter<>());
   boolean success = subtaskCheckpointCoordinator.notifyCheckpointComplete(newCheckpointId, chain, () -> false);
   
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] rkhachatryan commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#discussion_r408370536
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/state/TestTaskStateManager.java
 ##########
 @@ -46,6 +46,7 @@
 public class TestTaskStateManager implements TaskStateManager {
 
 	private long reportedCheckpointId;
+	private long notifiedCompletedCheckpointId;
 
 Review comment:
   Having a `List<Long>` instead of `long` here would make it useful in a broader set of cases.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [flink] rkhachatryan commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator

Posted by GitBox <gi...@apache.org>.
rkhachatryan commented on a change in pull request #11721: [FLINK-14652] Refactor notifyCheckpointComplete to SubtaskCheckpointCoordinator
URL: https://github.com/apache/flink/pull/11721#discussion_r408765813
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/state/TestTaskStateManager.java
 ##########
 @@ -46,6 +46,7 @@
 public class TestTaskStateManager implements TaskStateManager {
 
 	private long reportedCheckpointId;
+	private long notifiedCompletedCheckpointId;
 
 Review comment:
   Yes, I agree it's enough currently.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services