Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34662511
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala
---
@@ -306,99 +308,100 @@ extends Actor with
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34662919
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala
---
@@ -306,99 +308,100 @@ extends Actor with ActorLogMessages with
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34663165
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala
---
@@ -306,99 +308,100 @@ extends Actor with
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34662347
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/AccumulatorRegistry.java
---
@@ -0,0 +1,144 @@
+/*
+ * Licensed to
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34665546
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala
---
@@ -306,99 +308,100 @@ extends Actor with ActorLogMessages with
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34674005
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/AccumulatorRegistry.java
---
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/896
---
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
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-121605629
Thanks for the feedback @StephanEwen @uce. Will merge this once Travis
passes.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-121173161
Wasn't the rationale to have a separate message for the heartbeat and the
accumulators to keep the heartbeat messages small? What do you think? In any
case, I agree that it
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34547975
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/DataSet.java
---
@@ -408,14 +408,16 @@ public long count() throws Exception {
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34549555
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
---
@@ -400,13 +398,22 @@ class JobManager(protected val
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34552275
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/RuntimeContext.java
---
@@ -101,9 +101,9 @@
/**
* For system
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34547621
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/util/AbstractRuntimeUDFContext.java
---
@@ -53,32 +53,34 @@
private final
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34547538
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/RuntimeContext.java
---
@@ -101,9 +101,9 @@
/**
* For system
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34548050
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/AccumulatorEvent.java
---
@@ -18,32 +18,60 @@
package
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34504410
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
---
@@ -400,13 +398,22 @@ class JobManager(protected val
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34503354
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/RuntimeContext.java
---
@@ -101,9 +101,9 @@
/**
* For system
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34504087
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/AccumulatorEvent.java
---
@@ -18,32 +18,60 @@
package
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34503752
--- Diff: flink-java/src/main/java/org/apache/flink/api/java/DataSet.java
---
@@ -408,14 +408,16 @@ public long count() throws Exception {
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34503586
--- Diff:
flink-core/src/main/java/org/apache/flink/api/common/functions/util/AbstractRuntimeUDFContext.java
---
@@ -53,32 +53,34 @@
private final
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34503896
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/AccumulatorRegistry.java
---
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache
Github user uce commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-121046784
Nice piece of work! I agree with Stephan's points. I think it was good to
address them. :) I've added some minor comments inline.
Regarding the high-level comments:
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335824
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/Execution.java
---
@@ -131,6 +133,35 @@
private
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335830
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java ---
@@ -172,13 +173,20 @@
/** The library cache, from which the
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335963
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/AccumulatorRegistry.java
---
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335420
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala
---
@@ -1017,6 +1040,9 @@ object TaskManager {
val
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-120302065
Looks like this change breaks the YARN integration. The YARN WordCount no
longer works.
Should be working again now.
It would be good if the accumulator
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-120305327
The reporter object has the advantage that it is more easily extensible. At
some point we will want to differentiate between locally received bytes, and
remotely
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335325
--- Diff:
flink-examples/flink-java-examples/src/main/java/org/apache/flink/examples/java/wordcount/WordCount.java
---
@@ -60,6 +60,7 @@ public static void
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335345
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/runtime/io/StreamingAbstractRecordReader.java
---
@@ -57,6
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335337
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java ---
@@ -172,13 +173,20 @@
/** The library cache, from which the
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335380
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala
---
@@ -306,99 +308,100 @@ extends Actor with ActorLogMessages with
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335330
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/RuntimeEnvironment.java
---
@@ -212,19 +218,19 @@ public InputGate getInputGate(int
Github user mxm commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34335342
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java ---
@@ -237,12 +246,13 @@ public Task(TaskDeploymentDescriptor tdd,
Github user mxm commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-120460741
I've addressed your comments in a new commit.
---
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
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-120029960
Looks like this change breaks the YARN integration. The YARN WordCount no
longer works.
---
If your project is set up for it, you can reply to this email and have
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34268803
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/RuntimeEnvironment.java
---
@@ -212,19 +218,19 @@ public InputGate
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34266800
--- Diff:
flink-examples/flink-java-examples/src/main/java/org/apache/flink/examples/java/wordcount/WordCount.java
---
@@ -60,6 +60,7 @@ public static
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34269026
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java ---
@@ -237,12 +246,13 @@ public Task(TaskDeploymentDescriptor tdd,
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-120054333
The is a potential modification conflict: Drawing a snapshot for
serialization and registering a new accumulator can lead to a
ConcurrentModificationException in the
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34273441
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/Execution.java
---
@@ -601,6 +636,11 @@ void markFinished() {
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34274318
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java ---
@@ -172,13 +173,20 @@
/** The library cache, from
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34274983
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/accumulators/AccumulatorRegistry.java
---
@@ -0,0 +1,140 @@
+/*
+ * Licensed to
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34270017
--- Diff:
flink-staging/flink-streaming/flink-streaming-core/src/main/java/org/apache/flink/streaming/runtime/io/StreamingAbstractRecordReader.java
---
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/896#issuecomment-120058469
The naming of the accumulators refers sometimes to flink vs.
user-defined, and sometimes to internal vs. external. Can we make this
consistent? I actually like the
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34271710
--- Diff:
flink-examples/flink-java-examples/src/main/java/org/apache/flink/examples/java/wordcount/WordCount.java
---
@@ -60,6 +60,7 @@ public static
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34272101
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala
---
@@ -306,99 +308,100 @@ extends Actor with
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34273000
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/Execution.java
---
@@ -131,6 +133,35 @@
private
GitHub user mxm opened a pull request:
https://github.com/apache/flink/pull/896
[FLINK-2292][FLINK-1573] add live per-task accumulators
This refactors the accumulators to accumulate per task execution. The
accumulators are reported from the task managers periodically to the job
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34272308
--- Diff:
flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala
---
@@ -1017,6 +1040,9 @@ object TaskManager {
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/896#discussion_r34273136
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/Execution.java
---
@@ -601,6 +636,11 @@ void markFinished() {
51 matches
Mail list logo
