You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by HuangWHWHW <gi...@git.apache.org> on 2015/09/06 06:17:20 UTC
[GitHub] flink pull request: [FLINK-2626][CORE]add averageaccumulator
GitHub user HuangWHWHW opened a pull request:
https://github.com/apache/flink/pull/1096
[FLINK-2626][CORE]add averageaccumulator
Add class averageaccumulator for FLINK-accumulator.
And test for it.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HuangWHWHW/flink FLINK-2626
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/1096.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 #1096
----
commit 7064b080e991999ed6d599b4d2c630bbc50720c7
Author: HuangWHWHW <40...@qq.com>
Date: 2015-09-06T04:15:12Z
[FLINK-2626][CORE]add averageaccumulator
----
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by mjsax <gi...@git.apache.org>.
Github user mjsax commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138280298
Fail enough. Ignore my comment.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1096#discussion_r38856538
--- Diff: flink-core/src/test/java/org/apache/flink/api/common/accumulators/AverageAccumulatorTest.java ---
@@ -0,0 +1,117 @@
+/*
+ * 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.api.common.accumulators;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
--- End diff --
Sorry...comment?
You mean this class documentation----'/*XXX*/'?
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by mjsax <gi...@git.apache.org>.
Github user mjsax commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138274145
Why is `public void add(Double value)` overloaded with `public void add(double value)`? The usage of primitive types and there class counterparts seem to be mixed... Why?
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1096#discussion_r38856794
--- Diff: flink-core/src/test/java/org/apache/flink/api/common/accumulators/AverageAccumulatorTest.java ---
@@ -0,0 +1,117 @@
+/*
+ * 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.api.common.accumulators;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
--- End diff --
Ok, I'll remove it.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by mjsax <gi...@git.apache.org>.
Github user mjsax commented on a diff in the pull request:
https://github.com/apache/flink/pull/1096#discussion_r38856253
--- Diff: flink-core/src/test/java/org/apache/flink/api/common/accumulators/AverageAccumulatorTest.java ---
@@ -0,0 +1,117 @@
+/*
+ * 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.api.common.accumulators;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
--- End diff --
This is obvious. Please remove comment.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by mjsax <gi...@git.apache.org>.
Github user mjsax commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138302632
+1 for 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138230933
@sachingoel0101
Hi,
I removed these two methods as you said.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138276764
@mjsax
Do you mean that remove the `public void add(double value)`?
I`m not quite sure how this can be used in the feature.
@StephanEwen
Thank you!
Otherwise, Can you take a look about my earlier PRs?
Since I have been fixing those for a long time and recently I took fixes and passed the CI of them.
Many thanks for you and sorry for wasting your time.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138273532
Looks good, +1 to merge this
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138277626
@mjsax , primitive add methods for accumulators were introduced in #942 to get rid of potential unnecessary boxing and unboxing.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138291760
@StephanEwen
Just a minor thing:
Does the zk do re-connection if it failed to set up a node or cannot receive the msg from any node?
As I know the zk is unstable in some cases and I have no idea about what cases.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by mjsax <gi...@git.apache.org>.
Github user mjsax commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138281436
```
Tests in error:
SlotCountExceedingParallelismTest.testNoSlotSharingAndBlockingResult » JobExecution
```
and
```
Failed tests:
KafkaITCase>KafkaTestBase.prepare:155 Test setup failed: Unable to connect to zookeeper server within timeout: 6000
KafkaProducerITCase>KafkaTestBase.prepare:155 Test setup failed: Unable to connect to zookeeper server within timeout: 6000
Tests in error:
KafkaITCase>KafkaTestBase.shutDownServices:196 » NullPointer
KafkaProducerITCase>KafkaTestBase.shutDownServices:196 » NullPointer
```
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138282731
@sachingoel0101
Thank you for spending time in explaining.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by mjsax <gi...@git.apache.org>.
Github user mjsax commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138277939
Yes, I don't see any advantage in having this overload. I would also change `add(long value)` and `add(int value)` into `add(Long value)` and `add(Integer value)` for consistency (I actually prefer primitive types, too; however, for this case class types seems to be more appropriate because the inherited methods use `Double` as parameter and as return type in `getLocalValue()`).
But you should wait for @StephanEwen feedback before chancing anything. I am new to accumulators...
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138279804
@mjsax, I'm not sure what's the need for those. The `add(Double)` is only needed because the interface requires this. `add(Long)` is equivalent to `add(long)` after unboxing, which is exactly what would happen inside the implementation of `add(Long)`.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/1096
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by mjsax <gi...@git.apache.org>.
Github user mjsax commented on a diff in the pull request:
https://github.com/apache/flink/pull/1096#discussion_r38856700
--- Diff: flink-core/src/test/java/org/apache/flink/api/common/accumulators/AverageAccumulatorTest.java ---
@@ -0,0 +1,117 @@
+/*
+ * 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.api.common.accumulators;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+/**
--- 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-2626][CORE]add averageaccumulator
Posted by mjsax <gi...@git.apache.org>.
Github user mjsax commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138288718
We should discuss this over the dev list.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on a diff in the pull request:
https://github.com/apache/flink/pull/1096#discussion_r38838895
--- Diff: flink-core/src/main/java/org/apache/flink/api/common/accumulators/AverageAccumulator.java ---
@@ -0,0 +1,98 @@
+/*
+ * 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.api.common.accumulators;
+
+/**
+ * An accumulator that get the average values.
+ * Input can be {@code long}, {@code integer}, {@code double} as the result is {@code double}.
+ */
+public class AverageAccumulator implements SimpleAccumulator <Double> {
+
+ private double localValue = 0;
+ private long count = 0;
+
+ @Override
+ public void add(Double value) {
+ this.count++;
+ this.localValue += value;
+ }
+
+ public void add(double value) {
+ this.count++;
+ this.localValue += value;
+ }
+
+ public void add(Long value) {
+ this.count++;
+ this.localValue += (double)value;
+ }
+
+ public void add(long value) {
+ this.count++;
+ this.localValue += value;
+ }
+
+ public void add(Integer value) {
--- End diff --
Same as above.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138287360
@mjsax
Otherwise, since this accumulator is prepared for the cust metrics in storm-compatibility.
Can you give me a storm example in using the cust metrics?
Or is it more suitable to send a email to you for this question on dev mailing list?
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138293709
I think you can cinfigure ZooKeeper to do that.
The problem in that test was that the Kafka server has an initial timeout when trying to connect to ZooKeeper. Here, ZooKeeper is an embedded mini-cluster-zookeeper for tests. Apparently either that mini cluster did not start properly, or the build was accidentally very slow (it happens in these shared VM environments) so that the timeout triggered.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on a diff in the pull request:
https://github.com/apache/flink/pull/1096#discussion_r38839638
--- Diff: flink-core/src/main/java/org/apache/flink/api/common/accumulators/AverageAccumulator.java ---
@@ -0,0 +1,98 @@
+/*
+ * 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.api.common.accumulators;
+
+/**
+ * An accumulator that get the average values.
+ * Input can be {@code long}, {@code integer}, {@code double} as the result is {@code double}.
+ */
+public class AverageAccumulator implements SimpleAccumulator <Double> {
+
+ private double localValue = 0;
+ private long count = 0;
+
+ @Override
+ public void add(Double value) {
+ this.count++;
+ this.localValue += value;
+ }
+
+ public void add(double value) {
+ this.count++;
+ this.localValue += value;
+ }
+
+ public void add(Long value) {
--- End diff --
That's required by the interface, just as `add(Double)` is required here.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on a diff in the pull request:
https://github.com/apache/flink/pull/1096#discussion_r38839459
--- Diff: flink-core/src/main/java/org/apache/flink/api/common/accumulators/AverageAccumulator.java ---
@@ -0,0 +1,98 @@
+/*
+ * 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.api.common.accumulators;
+
+/**
+ * An accumulator that get the average values.
+ * Input can be {@code long}, {@code integer}, {@code double} as the result is {@code double}.
+ */
+public class AverageAccumulator implements SimpleAccumulator <Double> {
+
+ private double localValue = 0;
+ private long count = 0;
+
+ @Override
+ public void add(Double value) {
+ this.count++;
+ this.localValue += value;
+ }
+
+ public void add(double value) {
+ this.count++;
+ this.localValue += value;
+ }
+
+ public void add(Long value) {
--- End diff --
I`m not quite sure.
Just the same as the class LongCounter .
You can see the code following:
public class LongCounter implements SimpleAccumulator<Long> {
private static final long serialVersionUID = 1L;
private long localValue = 0;
/**
* Consider using {@link #add(long)} instead for primitive long values
*/
@Override
public void add(Long value) {
this.localValue += value;
}
public void add(long value){
this.localValue += value;
}
@Override
public Long getLocalValue() {
return this.localValue;
}
@Override
public void merge(Accumulator<Long, Long> other) {
this.localValue += other.getLocalValue();
}
@Override
public void resetLocal() {
this.localValue = 0;
}
@Override
public LongCounter clone() {
LongCounter result = new LongCounter();
result.localValue = localValue;
return result;
}
@Override
public String toString() {
return "LongCounter " + this.localValue;
}
}
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138282630
@mjsax
Thank you for the info.
I take a fix about your comment and rerun the CI.
As for the error, I have no idea.
Is there any bugs in other module?
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138289016
I am pushing a hotfix to increase timeouts in the Kafka tests. That will hopefully make the Kafka tests more reliable on Travis
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138295489
@StephanEwen
Thank you.
I'll take learning in your further hotfix.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by HuangWHWHW <gi...@git.apache.org>.
Github user HuangWHWHW commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138280490
@sachingoel0101
@mjsax
Otherwise, could you tell why the CI is failed?
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by sachingoel0101 <gi...@git.apache.org>.
Github user sachingoel0101 commented on a diff in the pull request:
https://github.com/apache/flink/pull/1096#discussion_r38838868
--- Diff: flink-core/src/main/java/org/apache/flink/api/common/accumulators/AverageAccumulator.java ---
@@ -0,0 +1,98 @@
+/*
+ * 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.api.common.accumulators;
+
+/**
+ * An accumulator that get the average values.
+ * Input can be {@code long}, {@code integer}, {@code double} as the result is {@code double}.
+ */
+public class AverageAccumulator implements SimpleAccumulator <Double> {
+
+ private double localValue = 0;
+ private long count = 0;
+
+ @Override
+ public void add(Double value) {
+ this.count++;
+ this.localValue += value;
+ }
+
+ public void add(double value) {
+ this.count++;
+ this.localValue += value;
+ }
+
+ public void add(Long value) {
--- End diff --
Why do we need this? `add(long)` should suffice.
---
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: [FLINK-2626][CORE]add averageaccumulator
Posted by mjsax <gi...@git.apache.org>.
Github user mjsax commented on the pull request:
https://github.com/apache/flink/pull/1096#issuecomment-138283474
To me, the tests seems to be unstable. Maybe we need to open a JIRA for them. I will report them on the dev mailing list.
---
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.
---