Skip to content

Conversation

@bowenli86
Copy link
Member

What is the purpose of the change

When running WindowWordCount example with no params (using default params), no output is generated and thus printed, because the default 'window' and 'slide' value is too large (250 and 150).

The solution is to lower default 'window' and 'slide' values to probably 4 and 2

Brief change log

Lower default 'window' and 'slide' values in this example to 4 and 2

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

I tested the example with updated default values, and it now generates and prints output.

Does this pull request potentially affect one of the following parts:

Documentation

  • Does this pull request introduce a new feature? (no)

@zentol
Copy link
Contributor

zentol commented Sep 11, 2017

Aren't these values a bit too low? (I'm specifically worried about cases where users supply their own data)
Would 100/50 maybe work?

@bowenli86
Copy link
Member Author

bowenli86 commented Sep 11, 2017

There's no output with 100/50 either, because the default text we use at WordCountData.WORDS is not very long and cannot satisfy the count window size. There's only 11 window ouputs even with 10/5, which are still a bigger size. And I'm not in favor of adding more text to WordCountData.WORDS.

I prefer 4/2, but also think 10/5 is also ok, because the program is for demo purpose and new users would doubt the program's accuracy if it doesn't generate enough output. What do you think? @zentol

@zentol
Copy link
Contributor

zentol commented Sep 13, 2017

lets go with 10/5 then.

@bowenli86
Copy link
Member Author

@zentol Done

@bowenli86
Copy link
Member Author

@zentol and this one. Thanks!

@zentol
Copy link
Contributor

zentol commented Sep 19, 2017

merging.

zentol pushed a commit to zentol/flink that referenced this pull request Sep 19, 2017
@asfgit asfgit closed this in baebbab Sep 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants