[Minor][DOC] Add JavaStreamingTestExample #11776

zhengruifeng · 2016-03-17T06:41:08Z

What changes were proposed in this pull request?

Add the java example of StreamingTest

How was this patch tested?

manual tests in CLI: bin/run-example mllib.JavaStreamingTestExample dataDir 5 100

SparkQA · 2016-03-17T06:54:37Z

Test build #53399 has finished for PR 11776 at commit ff56ff5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

MLnick · 2016-03-17T07:32:51Z

examples/src/main/java/org/apache/spark/examples/mllib/JavaStreamingTestExample.java

+import org.apache.spark.api.java.function.VoidFunction;
+import org.apache.spark.api.java.JavaRDD;
+import org.apache.spark.api.java.function.Function;
+import org.apache.spark.mllib.stat.test.BinarySample;


Please add // $example on$ and // $example off$ for the required imports.

@MLnick OK, I add required imports according to your comments

SparkQA · 2016-03-17T08:25:13Z

Test build #53405 has finished for PR 11776 at commit 7750299.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

MLnick · 2016-03-17T08:33:43Z

examples/src/main/java/org/apache/spark/examples/mllib/JavaStreamingTestExample.java

+
+import org.apache.spark.Accumulator;
+// $example on$
+import org.apache.spark.api.java.function.VoidFunction;


Don't think this import is actually required, as that code is after the final $example off$ ?

@MLnick Thinks, I will remove it.

SparkQA · 2016-03-17T08:54:33Z

Test build #53409 has finished for PR 11776 at commit f251229.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

MLnick · 2016-03-17T09:12:04Z

Merged to master. Thanks!

srowen · 2016-03-17T09:34:15Z

examples/src/main/java/org/apache/spark/examples/mllib/JavaStreamingTestExample.java

+
+        long cntSignificant = rdd.filter(new Function<StreamingTestResult, Boolean>() {
+          @Override
+          public Boolean call(StreamingTestResult v) throws Exception {


There are some minor style sub-optimalities here, like some indentation issues, unneeded "throws Exception", and some variances from the Scala example. No big deal but this could have been left open more than a couple hours to get some eyes on it.

Sorry Sean - I admit I made a fairly quick pass. What variances from the Scala example do you see?

Ex: I see why this call doesn't use "fold" but could have been !rdd.filter(...).isEmpty; you don't need an arg to createTempDir; timeoutCounter actually shouldn't be an accumulator here. The only one that isn't trivial is the last one.

Utils.createTempDir uses default args so args are required in Java.

Ah right, scratch that one. The only thing that might be an issue here is counting with the accumulator. It's all actually local, so, I'm guessing it works OK anyway. It could be an AtomicInteger or anything that can be decremented and referred to inside the function, which will only ever run on the driver.

Fair enough - if one tries to just use a local var similar to the Scala example it won't compile as it needs a final var, maybe this was the workaround. Agreed AtomicInteger is cleaner. Happy to clean that up

zhengruifeng · 2016-03-18T01:32:46Z

@MLnick @srowen What should I do to refine the PR?

MLnick · 2016-03-18T04:59:15Z

I think you can actually just make that variable a static int outside the
main definition, and -= it in the foreachRDD block. Similar to Scala.

I tested it and it works - no need even for an atomicinteger.
On Fri, 18 Mar 2016 at 03:33, Ruifeng Zheng [email protected]
wrote:

@MLnick https://github.com/MLnick @srowen https://github.com/srowen
What should I do to refine the PR?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#11776 (comment)

zhengruifeng · 2016-03-18T10:49:19Z

@MLnick How can I fix it in this PR? Or I have to open another one?

MLnick · 2016-03-18T10:54:15Z

You'll need to open another one I'm afraid.

On Fri, 18 Mar 2016 at 12:50 Ruifeng Zheng [email protected] wrote:

@MLnick https://github.com/MLnick How can I fix it in this PR? Or I
have to open another one?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#11776 (comment)

zhengruifeng · 2016-03-18T11:06:02Z

@MLnick ok I will open another pr.

## What changes were proposed in this pull request? Fix some nits discussed in #11776 (comment) use !rdd.isEmpty instead of rdd.count > 0 use static instead of AtomicInteger remove unneeded "throws Exception" ## How was this patch tested? manual tests Author: Zheng RuiFeng <[email protected]> Closes #11821 from zhengruifeng/je_fix.

## What changes were proposed in this pull request? Add the java example of StreamingTest ## How was this patch tested? manual tests in CLI: bin/run-example mllib.JavaStreamingTestExample dataDir 5 100 Author: Zheng RuiFeng <[email protected]> Closes apache#11776 from zhengruifeng/streaming_je.

## What changes were proposed in this pull request? Fix some nits discussed in apache#11776 (comment) use !rdd.isEmpty instead of rdd.count > 0 use static instead of AtomicInteger remove unneeded "throws Exception" ## How was this patch tested? manual tests Author: Zheng RuiFeng <[email protected]> Closes apache#11821 from zhengruifeng/je_fix.

zhengruifeng added 4 commits March 17, 2016 09:52

create st_je

73b6da5

fix nits

c10d5be

add to md

3458aca

add to md

ff56ff5

MLnick reviewed Mar 17, 2016
View reviewed changes

add required imports

7750299

MLnick reviewed Mar 17, 2016
View reviewed changes

remove unnecessary imports

f251229

asfgit closed this in 204c9de Mar 17, 2016

zhengruifeng deleted the streaming_je branch March 17, 2016 09:15

srowen reviewed Mar 17, 2016
View reviewed changes

zhengruifeng mentioned this pull request Mar 18, 2016

[Minor][DOC] Fix nits in JavaStreamingTestExample #11821

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Minor][DOC] Add JavaStreamingTestExample #11776

[Minor][DOC] Add JavaStreamingTestExample #11776

zhengruifeng commented Mar 17, 2016

SparkQA commented Mar 17, 2016

MLnick Mar 17, 2016

zhengruifeng Mar 17, 2016

SparkQA commented Mar 17, 2016

MLnick Mar 17, 2016

zhengruifeng Mar 17, 2016

SparkQA commented Mar 17, 2016

MLnick commented Mar 17, 2016

srowen Mar 17, 2016

MLnick Mar 17, 2016

srowen Mar 17, 2016

MLnick Mar 17, 2016

srowen Mar 17, 2016

MLnick Mar 17, 2016

zhengruifeng commented Mar 18, 2016

MLnick commented Mar 18, 2016

zhengruifeng commented Mar 18, 2016

MLnick commented Mar 18, 2016

zhengruifeng commented Mar 18, 2016

[Minor][DOC] Add JavaStreamingTestExample #11776

[Minor][DOC] Add JavaStreamingTestExample #11776

Conversation

zhengruifeng commented Mar 17, 2016

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented Mar 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Mar 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Mar 17, 2016

MLnick commented Mar 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhengruifeng commented Mar 18, 2016

MLnick commented Mar 18, 2016

zhengruifeng commented Mar 18, 2016

MLnick commented Mar 18, 2016

zhengruifeng commented Mar 18, 2016