Migrate portable runners to second version of data channel code #25104

lukecwik · 2023-01-20T18:53:17Z

The deletion of Dataflow JRH allows us to follow-up with further clean-up as the majority of what was preventing this migration was an awkward re-use of the first version of the data channel code in the JRH.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
Update CHANGES.md with noteworthy changes.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

codecov · 2023-01-20T19:29:12Z

Codecov Report

Merging #25104 (4dad3c6) into master (b3aa2e8) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 4dad3c6 differs from pull request most recent head fe7f9c9. Consider uploading reports for the commit fe7f9c9 to get more accurate results

@@           Coverage Diff           @@
##           master   #25104   +/-   ##
=======================================
  Coverage   73.12%   73.12%           
=======================================
  Files         735      735           
  Lines       98161    98161           
=======================================
+ Hits        71782    71783    +1     
+ Misses      25016    25015    -1     
  Partials     1363     1363

Flag	Coverage Δ
python	`82.67% <0.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...python/apache_beam/runners/worker/worker_status.py	`74.66% <0.00%> (-0.67%)`	⬇️
...hon/apache_beam/runners/worker/bundle_processor.py	`93.30% <0.00%> (-0.25%)`	⬇️
...on/apache_beam/runners/dataflow/dataflow_runner.py	`81.74% <0.00%> (-0.15%)`	⬇️
...ks/python/apache_beam/runners/worker/sdk_worker.py	`89.24% <0.00%> (+0.16%)`	⬆️
...che_beam/runners/interactive/interactive_runner.py	`91.77% <0.00%> (+1.26%)`	⬆️
.../python/apache_beam/testing/test_stream_service.py	`92.85% <0.00%> (+4.76%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

github-actions · 2023-01-23T21:05:36Z

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @kennknowles for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

…e on the runner side This allows us to cleanup the existing code base now that the Dataflow JRH is gone and should improve the stability and performance for Java based portable runners built on top of this code since we have had a lot more use of the SDK harness variants at scale.

lukecwik · 2023-01-23T21:56:51Z

Run XVR_Flink PostCommit

lukecwik · 2023-01-23T21:57:00Z

Run Go Flink ValidatesRunner

lukecwik · 2023-01-23T21:57:19Z

Run Python Flink ValidatesRunner

lukecwik · 2023-01-23T21:57:32Z

Run Python Samza ValidatesRunner

lukecwik · 2023-01-23T21:57:55Z

Run Python Spark ValidatesRunner

lukecwik · 2023-01-23T21:58:10Z

Run Java Flink PortableValidatesRunner Streaming

lukecwik · 2023-01-23T21:58:19Z

Run Java Samza PortableValidatesRunner

lukecwik · 2023-01-23T21:58:32Z

Run Java Spark PortableValidatesRunner Batch

lukecwik · 2023-01-23T21:58:45Z

Run Go Samza ValidatesRunner

lukecwik · 2023-01-23T21:59:03Z

Run Go Spark ValidatesRunner

lukecwik · 2023-01-23T23:18:57Z

R: @y1chi @apilloud

github-actions · 2023-01-23T23:27:58Z

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control

y1chi

LGTM

y1chi · 2023-01-24T00:32:29Z

.../java/fn-execution/src/main/java/org/apache/beam/sdk/fn/data/BeamFnDataGrpcMultiplexer2.java

    }

    @Override
    public void onCompleted() {
      LOG.warn(
          "Hanged up for {}.",
          apiServiceDescriptor == null ? "unknown endpoint" : apiServiceDescriptor);
-      outboundObserver.onCompleted();
    }
  }
 }


Maybe we can rename this class to just BeamFnDataGrpcMultiplexer with a follow up?

Yeah, I wanted to do that in a separate CL since the commit history would be easier to understand.

#25144 is the follow-up

This is a follow-up for a comment in apache#25104 Also remove InboundDataClient which was missed in apache#25104

…r2 (#25144) This is a follow-up for a comment in #25104 Also remove InboundDataClient which was missed in #25104

Abacn · 2023-01-25T04:44:55Z

python postcommit tests apache_beam.io.external.xlang_debeziumio_it_test.CrossLanguageDebeziumIOTest.test_xlang_debezium_read start failing likely related to this change:

https://ci-beam.apache.org/view/PostCommit/job/beam_PostCommit_Python310/393/
https://ci-beam.apache.org/view/PostCommit/job/beam_PostCommit_Python39/1381/
https://ci-beam.apache.org/view/PostCommit/job/beam_PostCommit_Python38/3658/
https://ci-beam.apache.org/view/PostCommit/job/beam_PostCommit_Python37/ (same test failed though not shown in jenkins, can be checked from log)

Error Message
RuntimeError: Service failed to start up with error 1
Stacktrace
self = <apache_beam.io.external.xlang_debeziumio_it_test.CrossLanguageDebeziumIOTest testMethod=test_xlang_debezium_read>

looks like it fails to start the expansion service.

github-actions bot added build dataflow extensions fn-execution infra java runners zetasketch labels Jan 20, 2023

lukecwik force-pushed the beam13015.runnercleanup branch from 057e1ee to 007b6f9 Compare January 21, 2023 00:09

github-actions bot removed extensions zetasketch infra dataflow labels Jan 21, 2023

lukecwik force-pushed the beam13015.runnercleanup branch from 007b6f9 to fe7f9c9 Compare January 23, 2023 19:17

lukecwik changed the title ~~[WIP] Migrate portable runners to second version of data channel code~~ Migrate portable runners to second version of data channel code Jan 23, 2023

github-actions bot added Next Action: Reviewers and removed build labels Jan 23, 2023

lukecwik mentioned this pull request Jan 23, 2023

SDK should clean-up registered timer/data/... handles and not leak them with the BeamFnDataClient #21007

Open

lukecwik force-pushed the beam13015.runnercleanup branch from fe7f9c9 to 22ff99b Compare January 23, 2023 21:56

apache deleted a comment from github-actions bot Jan 23, 2023

y1chi approved these changes Jan 24, 2023

View reviewed changes

lukecwik merged commit 3d9318b into apache:master Jan 24, 2023

lukecwik added a commit to lukecwik/incubator-beam that referenced this pull request Jan 24, 2023

Drop '2' from BeamFnDataGrpcMultiplexer2 and BeamFnDataInboundObserver2

842750a

This is a follow-up for a comment in apache#25104 Also remove InboundDataClient which was missed in apache#25104

lukecwik mentioned this pull request Jan 24, 2023

Drop '2' from BeamFnDataGrpcMultiplexer2 and BeamFnDataInboundObserver2 #25144

Merged

3 tasks

lukecwik added a commit that referenced this pull request Jan 24, 2023

Drop '2' from BeamFnDataGrpcMultiplexer2 and BeamFnDataInboundObserve…

299a7b0

…r2 (#25144) This is a follow-up for a comment in #25104 Also remove InboundDataClient which was missed in #25104

This was referenced Jan 26, 2023

[Failing Test]: Python PostCommit failing xlang tests #25189

Closed

[Failing Test]: Go test TestDebeziumIO_BasicRead is consistency failing in beam_PostCommit_XVR_Direct test suite #25223

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate portable runners to second version of data channel code #25104

Migrate portable runners to second version of data channel code #25104

lukecwik commented Jan 20, 2023 •

edited

Loading

codecov bot commented Jan 20, 2023 •

edited

Loading

github-actions bot commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

github-actions bot commented Jan 23, 2023

y1chi left a comment

y1chi Jan 24, 2023

lukecwik Jan 24, 2023

lukecwik Jan 24, 2023

Abacn commented Jan 25, 2023

Migrate portable runners to second version of data channel code #25104

Migrate portable runners to second version of data channel code #25104

Conversation

lukecwik commented Jan 20, 2023 • edited Loading

GitHub Actions Tests Status (on master branch)

codecov bot commented Jan 20, 2023 • edited Loading

Codecov Report

github-actions bot commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

lukecwik commented Jan 23, 2023

github-actions bot commented Jan 23, 2023

y1chi left a comment

Choose a reason for hiding this comment

y1chi Jan 24, 2023

Choose a reason for hiding this comment

lukecwik Jan 24, 2023

Choose a reason for hiding this comment

lukecwik Jan 24, 2023

Choose a reason for hiding this comment

Abacn commented Jan 25, 2023

lukecwik commented Jan 20, 2023 •

edited

Loading

codecov bot commented Jan 20, 2023 •

edited

Loading