[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
GitHub user rmetzger opened a pull request: https://github.com/apache/flink/pull/895 [FLINK-2008][FLINK-2296] Fix checkpoint committing & KafkaITCase [wip] You can merge this pull request into a Git repository by running: $ git pull https://github.com/rmetzger/flink flink2008 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/895.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 #895 commit 0d5773a56acb9f6d7592de9bf5a93f04e6600ca1 Author: Robert Metzger Date: 2015-06-29T14:52:38Z [FLINK-2008][FLINK-2296] Fix checkpoint committing & KafkaITCase --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
Github user StephanEwen commented on a diff in the pull request: https://github.com/apache/flink/pull/895#discussion_r34261966 --- Diff: docs/apis/streaming_guide.md --- @@ -1300,6 +1300,9 @@ Another way of exposing user defined operator state for the Flink runtime for ch When the user defined function implements the `Checkpointed` interface, the `snapshotState(â¦)` and `restoreState(â¦)` methods will be executed to draw and restore function state. +In addition to that, user functions can also implement the `CheckpointNotifier` interface to receive notifications on completed checkpoints via the `notifyCheckpointComplete(long checkpointId)` method. +Note that there is no guarantee for the user function to receive a notification once the checkpoint is complete. --- End diff -- Let's write it like "Note that there is no guarantee for the user function to receive a notification if a failure happens between checkpoint completion and notification. The notifications should hence be treated in a way that notifications from later checkpoints can subsume missing notifications." --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
Github user StephanEwen commented on a diff in the pull request: https://github.com/apache/flink/pull/895#discussion_r34262084 --- Diff: flink-tests/src/test/resources/log4j-test.properties --- @@ -18,7 +18,7 @@ # Set root logger level to OFF to not flood build logs # set manually to INFO for debugging purposes -log4j.rootLogger=OFF, testlogger +log4j.rootLogger=INFO, testlogger --- End diff -- was this accidentally committed? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
Github user StephanEwen commented on a diff in the pull request: https://github.com/apache/flink/pull/895#discussion_r34262141 --- Diff: flink-tests/src/test/java/org/apache/flink/test/checkpointing/StreamCheckpointingITCase.java --- @@ -53,6 +62,8 @@ @SuppressWarnings("serial") public class StreamCheckpointingITCase { + private static final Logger LOG = LoggerFactory.getLogger(StreamCheckpointingITCase.class); --- End diff -- These changes here also seem accidentally committed --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/895#discussion_r34262145 --- Diff: flink-tests/src/test/resources/log4j-test.properties --- @@ -18,7 +18,7 @@ # Set root logger level to OFF to not flood build logs # set manually to INFO for debugging purposes -log4j.rootLogger=OFF, testlogger +log4j.rootLogger=INFO, testlogger --- End diff -- Yes --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/895#discussion_r34262391 --- Diff: flink-tests/src/test/java/org/apache/flink/test/checkpointing/StreamCheckpointingITCase.java --- @@ -53,6 +62,8 @@ @SuppressWarnings("serial") public class StreamCheckpointingITCase { + private static final Logger LOG = LoggerFactory.getLogger(StreamCheckpointingITCase.class); --- End diff -- Before I decided to undo parts of the checkpointing changes (sending the full state back from the JM to the tasks), I had added an additional test there. I can delete the logger factory. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
Github user StephanEwen commented on a diff in the pull request: https://github.com/apache/flink/pull/895#discussion_r34262472 --- Diff: flink-staging/flink-streaming/flink-streaming-connectors/flink-connector-kafka/src/main/java/org/apache/flink/streaming/connectors/kafka/Utils.java --- @@ -53,13 +62,22 @@ public boolean isEndOfStream(T nextElement) { @Override public byte[] serialize(T element) { - DataOutputSerializer dos = new DataOutputSerializer(16); + if(dos == null) { + dos = new DataOutputSerializer(1); --- End diff -- I think a starting size of `16` is nicer than one of `1` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
Github user StephanEwen commented on the pull request: https://github.com/apache/flink/pull/895#issuecomment-120065921 Except for the issue mentioned by Gyula (the double commit of the head), this looks good. I would like to merge this later today or tomorrow. Could address Gyula's comment while merging... --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
Github user gyfora commented on the pull request: https://github.com/apache/flink/pull/895#issuecomment-120421266 :+1: --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/895 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---