[GitHub] flink pull request: [FLINK-2008][FLINK-2296] Fix checkpoint commit...

2015-07-09 Thread rmetzger
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...

2015-07-09 Thread StephanEwen
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...

2015-07-09 Thread StephanEwen
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...

2015-07-09 Thread StephanEwen
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...

2015-07-09 Thread rmetzger
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...

2015-07-09 Thread rmetzger
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...

2015-07-09 Thread StephanEwen
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...

2015-07-09 Thread StephanEwen
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...

2015-07-10 Thread gyfora
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...

2015-07-14 Thread asfgit
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.
---