[BEAM-13614, BEAM-13213] Add OnWindowExpiration support to the Java SDK harness and proto translation by lukecwik · Pull Request #16458 · apache/beam

lukecwik · 2022-01-07T20:44:43Z

This implementation adds a timer family spec in the event time domain and adds the field to the ParDoPayload mentioning which timer family spec represents the on window expiration callback.

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

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
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.

`ValidatesRunner` compliance status (on master branch)

Lang	ULR	Twister2
Go	---	---
Java
Python	---	---
XLang		---

Examples testing status on various runners

Lang	ULR	Dataflow	Flink	Samza	Spark	Twister2
Go	---	---	---	---	---	---	---
Java	---		---	---	---	---	---
Python	---	---	---	---	---	---	---
XLang	---	---	---	---	---	---	---

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Go	Java	Python

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---			---	---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

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

lukecwik · 2022-01-07T20:46:03Z

R: @TheNeuralBit
CC: @reuvenlax @robertwb

codecov · 2022-01-07T21:07:29Z

Codecov Report

Merging #16458 (3cf8cf3) into master (fea9c4b) will increase coverage by 9.00%.
The diff coverage is n/a.

@@ Coverage Diff @@## master #16458 +/- ##
==========================================
+ Coverage 74.59% 83.59% +9.00% 
==========================================
Files 653 452 -201 Lines 81688 61986 -19702 ==========================================
- Hits 60932 51818 -9114 + Misses 19777 10168 -9609 + Partials 979 0 -979

Impacted Files	Coverage Δ
...python/apache_beam/runners/worker/worker_status.py	`78.26% <0.00%> (-1.45%)`	⬇️
...apache_beam/portability/api/beam_runner_api_pb2.py	`100.00% <0.00%> (ø)`
...e_beam/portability/api/beam_runner_api_pb2_urns.py	`100.00% <0.00%> (ø)`
sdks/go/pkg/beam/testing/passert/floats.go
sdks/go/pkg/beam/core/util/reflectx/types.go
sdks/go/pkg/beam/util/starcgenx/starcgenx.go
sdks/go/pkg/beam/schema.go
sdks/go/pkg/beam/core/runtime/harness/session.go
sdks/go/pkg/beam/provision/provision.go
sdks/go/pkg/beam/core/runtime/exec/fn_arity.go
... and 198 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fea9c4b...3cf8cf3. Read the comment docs.

lukecwik · 2022-01-07T22:50:58Z

Run Java PreCommit

lukecwik · 2022-01-08T00:06:26Z

Run Java PreCommit

…nd proto translation. This implementation adds a timer family spec in the event time domain and adds the field to the ParDoPayload mentioning which timer family spec represents the on window expiration callback.

lukecwik · 2022-01-10T19:17:55Z

R: @reuvenlax @laraschmidt

kennknowles

any validatesrunner tests to enable for portable runners?

kennknowles · 2022-01-10T19:43:07Z

+ try {
+ currentKey = timer.getUserKey();
+ currentTimer = timer;
+ Iterator<BoundedWindow> windowIterator =


for (BoundedWindow currentWindow : timer.getWindows()) { ... } ?

currentWindow is defined outside of the method. I believe for-each loops are limited to defining the variable within the for (...) so I can't do for (currentWindow : timer.getWindows()) { ... }

Ah yes, I see that you are mutating a field so that the nested instance can carry it to the invoker. Gotta say there's quite a lot of implicit data flows here. I understand it is mutation for efficiency. Might be worth thinking about how to make them more explicit and local.

lukecwik · 2022-01-10T19:49:26Z

any validatesrunner tests to enable for portable runners?

No, I didn't add support to any portable runner. The only e2e test of the SDK harness is the one in the RemoteExecutionTest.java

Adding support to any portable runner would be a good follow-up.

lukecwik · 2022-01-10T19:54:30Z

Run Portable_Python PreCommit

kennknowles · 2022-01-10T20:15:52Z

+ try {
+ currentKey = timer.getUserKey();
+ currentTimer = timer;
+ Iterator<BoundedWindow> windowIterator =


Ah yes, I see that you are mutating a field so that the nested instance can carry it to the invoker. Gotta say there's quite a lot of implicit data flows here. I understand it is mutation for efficiency. Might be worth thinking about how to make them more explicit and local.

lukecwik · 2022-01-10T21:01:26Z

Run Java PreCommit

lukecwik · 2022-01-10T22:36:18Z

Run Java PreCommit

robertwb · 2022-01-11T01:19:38Z

Note that runners should now start rejecting these jobs due to the new requirement even if they have no idea what this is, rather than possibly silently dropping data.

Is the is a candidate for a cherry-pick into the ongoing release?

lukecwik · 2022-01-11T21:09:50Z

@robertwb

I believe the current plan is to recut the RC by syncing to HEAD due to gradle upgrade and security vulnerability patching. If this was to change, @emilymye please cherry-pick this.

robertwb · 2022-01-11T21:34:10Z

Sounds good. I re-opened https://issues.apache.org/jira/browse/BEAM-13213 to make sure this shows up on the list of blockers to keep an eye on--we can close once it's actually in the release branch one way or another.

* github/master: (198 commits) Merge pull request apache#16369 from [BEAM-13558] [Playground] Hide the Graph tab and SCIO from SDK options Merge pull request apache#16546 from [BEAM-13661] [BEAM-13704] [Playground] Update tags for examples/katas/unit-tests Merge pull request apache#16540 from [BEAM-13678][Playground]Update Github Action To Deploy Examples [BEAM-13430] Re-add provided configuration (apache#16552) [BEAM-10206] Remove Fatalf calls in non-test goroutines for tests/benchmarks (apache#16575) [BEAM-13693] Bump beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming timeout to 12 hours (apache#16576) [BEAM-13699] Replace fnv with maphash. (apache#16573) Update Java FnAPI beam master (apache#16572) Merge pull request apache#16371 from [BEAM-13518][Playground] Beam Playground quickstart page on the Beam website Merge pull request apache#16373 from [BEAM-13515] [Playground] Hiding lines in an example that are not necessary Merge pull request apache#16472: [BEAM-13697] Add SchemaFieldNumber annotation [BEAM-13689] Output token elements when BQ batch writes complete. Disable logging for memoization test. (apache#16556) BEAM-13611 reactivating jdbcio xlang test Revert "Merge pull request apache#15863 from [BEAM-13184] Autosharding for JdbcIO.write* transforms" Remove obsolete commands from Inventory job. (apache#16564) [BEAM-13688] fixed type in BPG 4.5.3 window section (apache#16560) [BEAM-13683] Make cross-language SQL example pipeline (apache#16567) Merge pull request apache#16243 from darshan-sj/feature/support-priority-spannerio - Making rpcPriority a ValueProvider in SpannerConfig edited README and comments in Python multi-lang pipes examples Merge pull request apache#16518 from [BEAM-13619] [Playground] Add loading animation to the catalog Merge pull request apache#16519 from [BEAM-13639] [Playground] Add notification to Log/Output tabs about cached example Merge pull request apache#16533 from [BEAM-13548] [Playground] Add example description popover Merge pull request apache#16531 from [BEAM-13567] [playground] Handle run code validation and preparation errors Merge pull request apache#16370 from [BEAM-13556] playground - color and scroll tabs with new content [BEAM-13611] Skip test_xlang_jdbc_write (apache#16554) [BEAM-13015] Provide caching statistics in the status client. (apache#16495) Merge pull request apache#16309: [BEAM-13503] Set a default value to throwWriteErrors in BulkIO constructor [BEAM-13015] Add state caching capability to be used as hint for runners to not duplicate cached data if the SDK can do it for user state and side inputs. (apache#16525) [BEAM-13665] Make SpannerIO projectID optional again (apache#16547) Merge pull request apache#16322 from [BEAM-13407] [Playground] Preload fonts for the web application Merge pull request apache#16506 from [BEAM-13652][Playground] Send examples' links to the frontend [BEAM-11808][BEAM-9879] Support aggregate functions with two arguments (apache#16200) Update walkthrough.md (apache#16512) [BEAM-13683] Correct SQL transform schema, fix expansion address override bug (apache#16551) Merge pull request apache#16486 from [BEAM-13544][Playground] Add logs to examples CI/CD to see the progress [BEAM-13616][BEAM-13646] Upgrade vendored calcite to 1.28.0:0.2 (apache#16544) [BEAM-13616][BEAM-13645] Switch to vendored grpc 1.43.2 (apache#16543) Also bump FnAPI container. [BEAM-13680] Fixed code_repository (added pipelineUuid to RunCodeResult when status is "Finished") [BEAM-13616] Update com.google.cloud:libraries-bom to 24.2.0 (apache#16509) [BEAM-13430] Remove jcenter which will no longer contain any updates. (apache#16536) Update GH Actions to use proper variables names and proper triggers Remove jcenter repositories from gradle configuration. (apache#16532) Merge pull request apache#16507: [BEAM-13137] Fixes ES utest size flakiness with _flush api and index.store.stats_refresh_interval=0 [BEAM-13664] Fix Primitives hashing benchmark (apache#16523) Bump beam container version. [BEAM-12621] - Update Jenkins VMs to modern Ubuntu version (apache#16457) doc tweaks (apache#16498) Redirecting cross-language transforms content (apache#16504) Remove tab from source. Remove unnecessary fmt call in universal.go Clean up string cast of bytes in vet.go and corresponding tests fix capitalized error strings in expansionx Remove unnecessary blank identifier assignment in harness.go Replace string(buf.Bytes()) with buf.String() in coder_test.go Replace bytes.Compare() with bytes.Equal() in test cases Remove unnecessary fmt.Sprintf() in partition.go Fix staticcheck errors in transforms directory [BEAM-13590] Fix abc imports from collections (apache#15850) Merge pull request apache#16482 from [BEAM-13429][Playground] Add builder for preparers [BEAM-10206] Resolve go vet errors in protox package [BEAM-12572] Run java examples on multiple runners (apache#16450) [BEAM-13400] JDBC IO does not support UUID and JSONB PostgreSQL types and OTHER JDBC types in general [BEAM-13577] Beam Select's uniquifyNames function loses nullability of Complex types while inferring schema [BEAM-12164]: Add SDF for reading change stream records [BEAM-13455] Remove duplicated artifacts when using multiple environments with Dataflow Java Merge pull request apache#16485 from [BEAM-13486] [Playground] For unit tests (java) if one of tests fails the output goes to stdOutput Merge pull request apache#16385 from [BEAM-13535] [Playground] add cancel execution button [BEAM-12558] Fix doc typo. Merge pull request apache#16467 from [BEAM-12164]: SpannerIO DetectNewPartitions SDF Introduce the notion of a JoinIndex for fewer shuffles. (apache#16101) [BEAM-12464] Change ProtoSchemaTranslator beam schema creation to match the order for protobufs containing Oneof fields (apache#14974) Stronger typing inference for CoGBK. (apache#16465) [BEAM-13480] Increase pipeline timeout for PubSubIntegrationTest.test_streaming_data_only (apache#16496) Provide API to check whether a hint is known. [BEAM-8806] Integration test for SqsIO using Localstack Split builder into several builder for each step of pipeline execution [BEAM-13399] Move service liveness polling to Runner type (apache#16487) Merge pull request apache#16325 from [BEAM-13471] [Playground] Tag existing unit-tests Adds several example multi-language Python pipelines [BEAM-13616][BEAM-13646] Update vendored calcite 1.28.0 with protobuf 3.19.2 (apache#16473) Merge pull request apache#16374 from [BEAM-13398][Playground] Split LifeCycle to DTO and business logic Merge pull request apache#16363 from [BEAM-13557] [Playground] show code execution time Merge pull request apache#16149 from [BEAM-13113] [Playground] playground frontend documentation Merge pull request apache#16469 from [BEAM-13623][Playground] [Bugfix] During unit tests failing there is no any output [BEAM-13641][Playground] Add SCIO SDK support on the CI/CD step [BEAM-13638] Datatype of timestamp fields in SqsMessage for AWS IOs for SDK v2 was changed from String to long, visibility of all fields was fixed from package private to public [BEAM-13616] Initial files for vendored gRPC 1.43.2 (apache#16460) [BEAM-13432] Skip ExpansionService creation in Job Server (apache#16222) [BEAM-13628] Update SideInputCache to use full Transform and SideInputIDs as token information (apache#16483) [BEAM-13631] Add deterministic SQS message coder to fix reading from SQS in batch mode [BEAM-8806] Integration test for SqsIO [adhoc] Run spotlessApply on java examples to fix master Merge pull request apache#16156 from [BEAM-13391] Fix temporary file format in WriteToBigQuery Optional args and kwargs for named external transforms. [BEAM-13614] Add OnWindowExpiration support to the Java SDK harness and proto translation. (apache#16458) [BEAM-3221] Improve documentation in model pipeline protos (apache#16474) Merge pull request apache#16147 from [BEAM-13359] [Playground] Tag existing examples [BEAM-13626] Remap expanded outputs after merging. (apache#16471) ...

lukecwik force-pushed the beam13614 branch from 54d9554 to be2a63aCompare January 10, 2022 18:11

lukecwik changed the title ~~[BEAM-13614] Add OnWindowExpiration as a requirement so that portable runners can reject such jobs.~~[BEAM-13614] Add OnWindowExpiration support to the Java SDK harness and proto translationJan 10, 2022

lukecwik force-pushed the beam13614 branch from be2a63a to 2e4aad8Compare January 10, 2022 18:37

[BEAM-13614] Add OnWindowExpiration support to the Java SDK harness a…
3cf8cf3
…nd proto translation. This implementation adds a timer family spec in the event time domain and adds the field to the ParDoPayload mentioning which timer family spec represents the on window expiration callback.

lukecwik force-pushed the beam13614 branch from 2e4aad8 to 3cf8cf3Compare January 10, 2022 19:00

lukecwik changed the title ~~[BEAM-13614] Add OnWindowExpiration support to the Java SDK harness and proto translation~~[BEAM-13614, BEAM-13213] Add OnWindowExpiration support to the Java SDK harness and proto translationJan 10, 2022

kennknowles reviewed Jan 10, 2022
View reviewed changes

kennknowles approved these changes Jan 10, 2022
View reviewed changes

lukecwik merged commit 6390bcd into apache:masterJan 11, 2022

Uh oh!

Conversation

lukecwik commented Jan 7, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ValidatesRunner compliance status (on master branch)

Examples testing status on various runners

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

Uh oh!

lukecwik commented Jan 7, 2022

Uh oh!

codecovBot commented Jan 7, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lukecwik commented Jan 7, 2022

Uh oh!

lukecwik commented Jan 8, 2022

Uh oh!

lukecwik commented Jan 10, 2022

Uh oh!

kennknowles left a comment

Choose a reason for hiding this comment

Uh oh!

kennknowlesJan 10, 2022

Choose a reason for hiding this comment

Uh oh!

lukecwikJan 10, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kennknowlesJan 10, 2022

Choose a reason for hiding this comment

Uh oh!

lukecwik commented Jan 10, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukecwik commented Jan 10, 2022

Uh oh!

kennknowlesJan 10, 2022

Choose a reason for hiding this comment

Uh oh!

lukecwik commented Jan 10, 2022

Uh oh!

lukecwik commented Jan 10, 2022

Uh oh!

robertwb commented Jan 11, 2022

Uh oh!

lukecwik commented Jan 11, 2022

Uh oh!

robertwb commented Jan 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lukecwik commented Jan 7, 2022•
edited
Loading

`ValidatesRunner` compliance status (on master branch)

codecovBot commented Jan 7, 2022•
edited
Loading

lukecwikJan 10, 2022•
edited
Loading

lukecwik commented Jan 10, 2022•
edited
Loading