[jira] [Commented] (BEAM-1712) TestPipeline.run doesn't actually waitUntilFinish
[ https://issues.apache.org/jira/browse/BEAM-1712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15923722#comment-15923722 ] Aljoscha Krettek commented on BEAM-1712: [~jkff], just wanted to confirm that both Flink Runners are in fact always blocking. There's BEAM-593 for fixing this. > TestPipeline.run doesn't actually waitUntilFinish > - > > Key: BEAM-1712 > URL: https://issues.apache.org/jira/browse/BEAM-1712 > Project: Beam > Issue Type: Bug > Components: runner-flink, sdk-java-core, testing >Reporter: Eugene Kirpichov >Assignee: Stas Levin > > https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/testing/TestPipeline.java#L124 > it calls waitUntilFinish() only if > 1) run wasn't called > 2) enableAutoRunIfMissing is true. > However in practice both of these are false. > 1) run() is, in most tests, called. So effectively if you call .run() at all, > then this thing doesn't call waitUntilFinish(). > 2) enableAutoRunIfMissing() is set to true only via > TestPipeline.enableAutoRunIfMissing(), which is called only from its own unit > test. > This means that, for all tests that use TestPipeline - if the test waits > until finish, it's only because of the grace of the particular runner. Which > is like really bad. > We're lucky because in practice TestDataflowRunner, TestApexRunner, > TestSparkRunner in run() call themselves waitUntilFinish(). > However, TestFlinkRunner doesn't - i.e. there currently might be tests that > actually fail in Flink runner, undetected. > The proper fix to this is to fix TestPipeline to always waitUntilFinish(). > Currently testing a quick-fix in https://github.com/apache/beam/pull/2240 to > make sure Flink is safe. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (BEAM-1712) TestPipeline.run doesn't actually waitUntilFinish
[
https://issues.apache.org/jira/browse/BEAM-1712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15923650#comment-15923650
]
Stas Levin commented on BEAM-1712:
--
[A related
discussion|https://lists.apache.org/thread.html/abe8b48dfc9e1420b2cdcd6d74f136ab622c170c707751a182027163@%3Cdev.beam.apache.org%3E]
circulated on the dev-list some months ago.
Basically, I believe there isn't a consensus at the moment, as to whose
responsibility it is to call {{waitUntilFinish}}, if at all.
I believe this issue is related to
[BEAM-849|https://issues.apache.org/jira/browse/BEAM-849] and while I'm not
sure it's a pre-requisite, it sounds like reaching a point where we're happy
with the {{PipelineResult}} API would involve figuring out if and when
{{waitUntilFinish}} is to be called by clients (including {{TestPipeline}} and
the likes).
The [recent
thread|https://lists.apache.org/thread.html/86831496a08fe148e3b982cdb904f828f262c0b571543a9fed7b915d@%3Cdev.beam.apache.org%3E]
by [~jkff] brings up some good points, and hopefully can be used to make
progress here.
> TestPipeline.run doesn't actually waitUntilFinish
> -
>
> Key: BEAM-1712
> URL: https://issues.apache.org/jira/browse/BEAM-1712
> Project: Beam
> Issue Type: Bug
> Components: runner-flink, sdk-java-core, testing
>Reporter: Eugene Kirpichov
>Assignee: Stas Levin
>
> https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/testing/TestPipeline.java#L124
> it calls waitUntilFinish() only if
> 1) run wasn't called
> 2) enableAutoRunIfMissing is true.
> However in practice both of these are false.
> 1) run() is, in most tests, called. So effectively if you call .run() at all,
> then this thing doesn't call waitUntilFinish().
> 2) enableAutoRunIfMissing() is set to true only via
> TestPipeline.enableAutoRunIfMissing(), which is called only from its own unit
> test.
> This means that, for all tests that use TestPipeline - if the test waits
> until finish, it's only because of the grace of the particular runner. Which
> is like really bad.
> We're lucky because in practice TestDataflowRunner, TestApexRunner,
> TestSparkRunner in run() call themselves waitUntilFinish().
> However, TestFlinkRunner doesn't - i.e. there currently might be tests that
> actually fail in Flink runner, undetected.
> The proper fix to this is to fix TestPipeline to always waitUntilFinish().
> Currently testing a quick-fix in https://github.com/apache/beam/pull/2240 to
> make sure Flink is safe.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (BEAM-1712) TestPipeline.run doesn't actually waitUntilFinish
[ https://issues.apache.org/jira/browse/BEAM-1712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15923602#comment-15923602 ] Kenneth Knowles commented on BEAM-1712: --- Since TestPipeline is coupled with automatically configuring the runner for cross-runner tests, we cannot place limitations like "don't call waitToFinish()" or we can't write a cross-runner test that needs to waitToFinish(). Dually, we can't automatically call waitToFinish() or we can't write a cross-runner asynchronous test. We may not need one or both of these, but we'd need to be quite sure. > TestPipeline.run doesn't actually waitUntilFinish > - > > Key: BEAM-1712 > URL: https://issues.apache.org/jira/browse/BEAM-1712 > Project: Beam > Issue Type: Bug > Components: runner-flink, sdk-java-core, testing >Reporter: Eugene Kirpichov >Assignee: Stas Levin > > https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/testing/TestPipeline.java#L124 > it calls waitUntilFinish() only if > 1) run wasn't called > 2) enableAutoRunIfMissing is true. > However in practice both of these are false. > 1) run() is, in most tests, called. So effectively if you call .run() at all, > then this thing doesn't call waitUntilFinish(). > 2) enableAutoRunIfMissing() is set to true only via > TestPipeline.enableAutoRunIfMissing(), which is called only from its own unit > test. > This means that, for all tests that use TestPipeline - if the test waits > until finish, it's only because of the grace of the particular runner. Which > is like really bad. > We're lucky because in practice TestDataflowRunner, TestApexRunner, > TestSparkRunner in run() call themselves waitUntilFinish(). > However, TestFlinkRunner doesn't - i.e. there currently might be tests that > actually fail in Flink runner, undetected. > The proper fix to this is to fix TestPipeline to always waitUntilFinish(). > Currently testing a quick-fix in https://github.com/apache/beam/pull/2240 to > make sure Flink is safe. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (BEAM-1712) TestPipeline.run doesn't actually waitUntilFinish
[ https://issues.apache.org/jira/browse/BEAM-1712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15923351#comment-15923351 ] Eugene Kirpichov commented on BEAM-1712: As an intermediate step to unifying everything under TestPipeline calling waitUntilFinish, it'd be nice to make the current code less confusing at least: - TestPipeline should remove code that pretends that it calls waitUntilFinish - Test runners (TestBlahRunner) should document that they wait themselves - Tests should not manually call waitUntilFinish I.e. the responsibility should be centralized in some place; right now this place can only be individual test runners, and eventually it will be TestPipeline. > TestPipeline.run doesn't actually waitUntilFinish > - > > Key: BEAM-1712 > URL: https://issues.apache.org/jira/browse/BEAM-1712 > Project: Beam > Issue Type: Bug > Components: runner-flink, sdk-java-core, testing >Reporter: Eugene Kirpichov >Assignee: Stas Levin > > https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/testing/TestPipeline.java#L124 > it calls waitUntilFinish() only if > 1) run wasn't called > 2) enableAutoRunIfMissing is true. > However in practice both of these are false. > 1) run() is, in most tests, called. So effectively if you call .run() at all, > then this thing doesn't call waitUntilFinish(). > 2) enableAutoRunIfMissing() is set to true only via > TestPipeline.enableAutoRunIfMissing(), which is called only from its own unit > test. > This means that, for all tests that use TestPipeline - if the test waits > until finish, it's only because of the grace of the particular runner. Which > is like really bad. > We're lucky because in practice TestDataflowRunner, TestApexRunner, > TestSparkRunner in run() call themselves waitUntilFinish(). > However, TestFlinkRunner doesn't - i.e. there currently might be tests that > actually fail in Flink runner, undetected. > The proper fix to this is to fix TestPipeline to always waitUntilFinish(). > Currently testing a quick-fix in https://github.com/apache/beam/pull/2240 to > make sure Flink is safe. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (BEAM-1712) TestPipeline.run doesn't actually waitUntilFinish
[
https://issues.apache.org/jira/browse/BEAM-1712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15923347#comment-15923347
]
Kenneth Knowles commented on BEAM-1712:
---
I don't think it will yield correct behavior to always {{waitUntilFinish()}}
without considerable changes to some of the test runners / their
{{PipelineResult}} implementations. Seems like a good idea to try to simplify
and solidify, as there's also duplication.
> TestPipeline.run doesn't actually waitUntilFinish
> -
>
> Key: BEAM-1712
> URL: https://issues.apache.org/jira/browse/BEAM-1712
> Project: Beam
> Issue Type: Bug
> Components: runner-flink, sdk-java-core, testing
>Reporter: Eugene Kirpichov
>Assignee: Stas Levin
>
> https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/testing/TestPipeline.java#L124
> it calls waitUntilFinish() only if
> 1) run wasn't called
> 2) enableAutoRunIfMissing is true.
> However in practice both of these are false.
> 1) run() is, in most tests, called. So effectively if you call .run() at all,
> then this thing doesn't call waitUntilFinish().
> 2) enableAutoRunIfMissing() is set to true only via
> TestPipeline.enableAutoRunIfMissing(), which is called only from its own unit
> test.
> This means that, for all tests that use TestPipeline - if the test waits
> until finish, it's only because of the grace of the particular runner. Which
> is like really bad.
> We're lucky because in practice TestDataflowRunner, TestApexRunner,
> TestSparkRunner in run() call themselves waitUntilFinish().
> However, TestFlinkRunner doesn't - i.e. there currently might be tests that
> actually fail in Flink runner, undetected.
> The proper fix to this is to fix TestPipeline to always waitUntilFinish().
> Currently testing a quick-fix in https://github.com/apache/beam/pull/2240 to
> make sure Flink is safe.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (BEAM-1712) TestPipeline.run doesn't actually waitUntilFinish
[ https://issues.apache.org/jira/browse/BEAM-1712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15923329#comment-15923329 ] Eugene Kirpichov commented on BEAM-1712: OK, seems like that is in fact what happens. So this bug then is currently NOT a release blocker because, for various reasons, all test runners including Flink actually wait until finish even though TestPipeline doesn't ask them to. > TestPipeline.run doesn't actually waitUntilFinish > - > > Key: BEAM-1712 > URL: https://issues.apache.org/jira/browse/BEAM-1712 > Project: Beam > Issue Type: Bug > Components: runner-flink, sdk-java-core, testing >Reporter: Eugene Kirpichov >Assignee: Stas Levin >Priority: Blocker > > https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/testing/TestPipeline.java#L124 > it calls waitUntilFinish() only if > 1) run wasn't called > 2) enableAutoRunIfMissing is true. > However in practice both of these are false. > 1) run() is, in most tests, called. So effectively if you call .run() at all, > then this thing doesn't call waitUntilFinish(). > 2) enableAutoRunIfMissing() is set to true only via > TestPipeline.enableAutoRunIfMissing(), which is called only from its own unit > test. > This means that, for all tests that use TestPipeline - if the test waits > until finish, it's only because of the grace of the particular runner. Which > is like really bad. > We're lucky because in practice TestDataflowRunner, TestApexRunner, > TestSparkRunner in run() call themselves waitUntilFinish(). > However, TestFlinkRunner doesn't - i.e. there currently might be tests that > actually fail in Flink runner, undetected. > The proper fix to this is to fix TestPipeline to always waitUntilFinish(). > Currently testing a quick-fix in https://github.com/apache/beam/pull/2240 to > make sure Flink is safe. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (BEAM-1712) TestPipeline.run doesn't actually waitUntilFinish
[ https://issues.apache.org/jira/browse/BEAM-1712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15923326#comment-15923326 ] Eugene Kirpichov commented on BEAM-1712: An optimistic interpretation is probably that Flink runner in batch mode simply waits in .run(). Not sure about streaming mode then. > TestPipeline.run doesn't actually waitUntilFinish > - > > Key: BEAM-1712 > URL: https://issues.apache.org/jira/browse/BEAM-1712 > Project: Beam > Issue Type: Bug > Components: runner-flink, sdk-java-core, testing >Reporter: Eugene Kirpichov >Assignee: Stas Levin >Priority: Blocker > > https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/testing/TestPipeline.java#L124 > it calls waitUntilFinish() only if > 1) run wasn't called > 2) enableAutoRunIfMissing is true. > However in practice both of these are false. > 1) run() is, in most tests, called. So effectively if you call .run() at all, > then this thing doesn't call waitUntilFinish(). > 2) enableAutoRunIfMissing() is set to true only via > TestPipeline.enableAutoRunIfMissing(), which is called only from its own unit > test. > This means that, for all tests that use TestPipeline - if the test waits > until finish, it's only because of the grace of the particular runner. Which > is like really bad. > We're lucky because in practice TestDataflowRunner, TestApexRunner, > TestSparkRunner in run() call themselves waitUntilFinish(). > However, TestFlinkRunner doesn't - i.e. there currently might be tests that > actually fail in Flink runner, undetected. > The proper fix to this is to fix TestPipeline to always waitUntilFinish(). > Currently testing a quick-fix in https://github.com/apache/beam/pull/2240 to > make sure Flink is safe. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
[jira] [Commented] (BEAM-1712) TestPipeline.run doesn't actually waitUntilFinish
[ https://issues.apache.org/jira/browse/BEAM-1712?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15923321#comment-15923321 ] Eugene Kirpichov commented on BEAM-1712: https://github.com/apache/beam/blob/master/runners/flink/runner/src/main/java/org/apache/beam/runners/flink/FlinkRunnerResult.java#L86 seems to imply that Flink tests with PAssert are and always have been vacuous?... [~aljoscha] WDYT? > TestPipeline.run doesn't actually waitUntilFinish > - > > Key: BEAM-1712 > URL: https://issues.apache.org/jira/browse/BEAM-1712 > Project: Beam > Issue Type: Bug > Components: runner-flink, sdk-java-core, testing >Reporter: Eugene Kirpichov >Assignee: Stas Levin >Priority: Blocker > > https://github.com/apache/beam/blob/master/sdks/java/core/src/main/java/org/apache/beam/sdk/testing/TestPipeline.java#L124 > it calls waitUntilFinish() only if > 1) run wasn't called > 2) enableAutoRunIfMissing is true. > However in practice both of these are false. > 1) run() is, in most tests, called. So effectively if you call .run() at all, > then this thing doesn't call waitUntilFinish(). > 2) enableAutoRunIfMissing() is set to true only via > TestPipeline.enableAutoRunIfMissing(), which is called only from its own unit > test. > This means that, for all tests that use TestPipeline - if the test waits > until finish, it's only because of the grace of the particular runner. Which > is like really bad. > We're lucky because in practice TestDataflowRunner, TestApexRunner, > TestSparkRunner in run() call themselves waitUntilFinish(). > However, TestFlinkRunner doesn't - i.e. there currently might be tests that > actually fail in Flink runner, undetected. > The proper fix to this is to fix TestPipeline to always waitUntilFinish(). > Currently testing a quick-fix in https://github.com/apache/beam/pull/2240 to > make sure Flink is safe. -- This message was sent by Atlassian JIRA (v6.3.15#6346)
