You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apex.apache.org by ilooner <gi...@git.apache.org> on 2015/11/12 01:33:27 UTC
[GitHub] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
GitHub user ilooner opened a pull request:
https://github.com/apache/incubator-apex-core/pull/166
- APEX-129 #resolve #comment Fixed bug where tuples can be emitted ou…
…tside of a streaming window
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ilooner/incubator-apex-core APEX-129
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-apex-core/pull/166.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 #166
----
commit 4beab767f0e84fa07d1e8fcbfeb260bbaf2e749c
Author: Timothy Farkas <ti...@datatorrent.com>
Date: 2015-11-11T23:51:58Z
- APEX-129 #resolve #comment Fixed bug where tuples can be emitted outside of a streaming window
----
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/166#discussion_r45111099
--- Diff: engine/src/main/java/com/datatorrent/stram/engine/InputNode.java ---
@@ -145,7 +148,7 @@ else if (PROCESSING_MODE == ProcessingMode.EXACTLY_ONCE) {
ContainerStats.OperatorStats stats = new ContainerStats.OperatorStats();
reportStats(stats, currentWindowId);
- if(!insideWindow){
+ if(!insideApplicationWindow){
--- End diff --
@PramodSSImmaneni it was documented and shared with the team pre-Apex
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-apex-core/pull/166
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by 243826 <gi...@git.apache.org>.
Github user 243826 commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/166#issuecomment-157196138
No, please go ahead.
On Nov 16, 2015 2:11 PM, "Pramod Immaneni" <no...@github.com> wrote:
> @243826 <https://github.com/243826> do you have any other comments? I was
> planning on doing the final review.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-apex-core/pull/166#issuecomment-157187728>
> .
>
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by 243826 <gi...@git.apache.org>.
Github user 243826 commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/166#discussion_r44608578
--- Diff: engine/src/main/java/com/datatorrent/stram/engine/InputNode.java ---
@@ -69,24 +69,27 @@ public final void run()
long spinMillis = context.getValue(OperatorContext.SPIN_MILLIS);
final boolean handleIdleTime = operator instanceof IdleTimeHandler;
- boolean insideWindow = applicationWindowCount != 0;
+ boolean insideApplicationWindow = applicationWindowCount != 0;
boolean doCheckpoint = false;
+ boolean insideStreamingWindow = false;
try {
while (alive) {
Tuple t = controlTuples.sweep();
if (t == null) {
- if (insideWindow) {
+ if (insideApplicationWindow) {
int generatedTuples = 0;
--- End diff --
this and line 95 - can move inside of the streaming window block, right?
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/166#discussion_r45101851
--- Diff: engine/src/main/java/com/datatorrent/stram/engine/InputNode.java ---
@@ -145,7 +148,7 @@ else if (PROCESSING_MODE == ProcessingMode.EXACTLY_ONCE) {
ContainerStats.OperatorStats stats = new ContainerStats.OperatorStats();
reportStats(stats, currentWindowId);
- if(!insideWindow){
+ if(!insideApplicationWindow){
--- End diff --
@PramodSSImmaneni before auto metrics they were. but with auto metrics it was changed and that discussion happened and was discussed.
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by PramodSSImmaneni <gi...@git.apache.org>.
Github user PramodSSImmaneni commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/166#discussion_r44608737
--- Diff: engine/src/main/java/com/datatorrent/stram/engine/InputNode.java ---
@@ -69,24 +69,27 @@ public final void run()
long spinMillis = context.getValue(OperatorContext.SPIN_MILLIS);
final boolean handleIdleTime = operator instanceof IdleTimeHandler;
- boolean insideWindow = applicationWindowCount != 0;
+ boolean insideApplicationWindow = applicationWindowCount != 0;
boolean doCheckpoint = false;
+ boolean insideStreamingWindow = false;
try {
while (alive) {
Tuple t = controlTuples.sweep();
if (t == null) {
- if (insideWindow) {
+ if (insideApplicationWindow) {
--- End diff --
This should be insideStreamingWindow as well. For example handleIdleTime should not be called between streaming window boundaries.
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by PramodSSImmaneni <gi...@git.apache.org>.
Github user PramodSSImmaneni commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/166#issuecomment-157187728
@243826 do you have any other comments? I was planning on doing the final review.
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by PramodSSImmaneni <gi...@git.apache.org>.
Github user PramodSSImmaneni commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/166#discussion_r44997624
--- Diff: engine/src/main/java/com/datatorrent/stram/engine/InputNode.java ---
@@ -145,7 +148,7 @@ else if (PROCESSING_MODE == ProcessingMode.EXACTLY_ONCE) {
ContainerStats.OperatorStats stats = new ContainerStats.OperatorStats();
reportStats(stats, currentWindowId);
- if(!insideWindow){
+ if(!insideApplicationWindow){
--- End diff --
Can't wait till end of application window to collect stats
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by ilooner <gi...@git.apache.org>.
Github user ilooner commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/166#issuecomment-156268545
@PramodSSImmaneni @243826 I have added unit tests, I verified that the unit tests failed without the fix, and they now pass with the fix. Please review and merge
Thanks
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by PramodSSImmaneni <gi...@git.apache.org>.
Github user PramodSSImmaneni commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/166#discussion_r45100787
--- Diff: engine/src/main/java/com/datatorrent/stram/engine/InputNode.java ---
@@ -145,7 +148,7 @@ else if (PROCESSING_MODE == ProcessingMode.EXACTLY_ONCE) {
ContainerStats.OperatorStats stats = new ContainerStats.OperatorStats();
reportStats(stats, currentWindowId);
- if(!insideWindow){
+ if(!insideApplicationWindow){
--- End diff --
@chandnisingh They were being collected at end of streaming window before.
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by ilooner <gi...@git.apache.org>.
Github user ilooner commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/166#issuecomment-155958097
will add a unit test
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by chandnisingh <gi...@git.apache.org>.
Github user chandnisingh commented on a diff in the pull request:
https://github.com/apache/incubator-apex-core/pull/166#discussion_r44999231
--- Diff: engine/src/main/java/com/datatorrent/stram/engine/InputNode.java ---
@@ -145,7 +148,7 @@ else if (PROCESSING_MODE == ProcessingMode.EXACTLY_ONCE) {
ContainerStats.OperatorStats stats = new ContainerStats.OperatorStats();
reportStats(stats, currentWindowId);
- if(!insideWindow){
+ if(!insideApplicationWindow){
--- End diff --
Metrics are collected at the end of application window. I think Tim has just changed the name of the variable and nothing else has changed. @PramodSSImmaneni please go through older discussions about why Metrics are collected at the end of application window. Also I have an improvement where it phase of metrics collections can be specified.
---
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] incubator-apex-core pull request: - APEX-129 #resolve #comment Fix...
Posted by 243826 <gi...@git.apache.org>.
Github user 243826 commented on the pull request:
https://github.com/apache/incubator-apex-core/pull/166#issuecomment-155958018
can you add a unit test?
---
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.
---