[BEAM-10212] Integrate caching client #15214

anthonyqzhu · 2021-07-23T22:07:14Z

Adds state cache and caching client to process bundle execution

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.

anthonyqzhu · 2021-07-23T22:07:43Z

R: @amaliujia

codecov · 2021-07-26T19:15:37Z

Codecov Report

Merging #15214 (0669dea) into master (1117f7d) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 0669dea differs from pull request most recent head 170a7a6. Consider uploading reports for the commit 170a7a6 to get more accurate results

@@            Coverage Diff             @@
##           master   #15214      +/-   ##
==========================================
- Coverage   83.82%   83.82%   -0.01%     
==========================================
  Files         441      441              
  Lines       59706    59706              
==========================================
- Hits        50051    50048       -3     
- Misses       9655     9658       +3

Impacted Files	Coverage Δ
sdks/python/apache_beam/io/source_test_utils.py	`88.47% <0.00%> (-1.39%)`	⬇️
sdks/python/apache_beam/transforms/util.py	`95.81% <0.00%> (-0.17%)`	⬇️
...ks/python/apache_beam/runners/worker/sdk_worker.py	`88.85% <0.00%> (-0.16%)`	⬇️
...hon/apache_beam/runners/worker/bundle_processor.py	`93.51% <0.00%> (+0.24%)`	⬆️

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 1117f7d...170a7a6. Read the comment docs.

amaliujia · 2021-07-27T17:39:06Z

Overall LGTM. Please verify if the ordering timer test works.

amaliujia · 2021-08-05T17:57:46Z

Run Java_Examples_Dataflow PreCommit

amaliujia · 2021-08-05T17:58:03Z

Run Java_Examples_Dataflow_Java11 PreCommit

amaliujia · 2021-08-05T17:59:53Z

sdks/java/harness/src/main/java/org/apache/beam/fn/harness/control/ProcessBundleHandler.java

+    // specified. If pipeline is batch, use a CachingBeamFnStateClient to store state responses.
+    // User state caching is currently not supported in streaming mode.
+    HandleStateCallsForBundle beamFnStateClient;
+    if (bundleDescriptor.hasStateApiServiceDescriptor()) {


Can you explain a bit more why hasStateApiServiceDescriptor is true = batch?

hasStateApiServiceDescriptor determines if a state handler is used at all, and options.as(StreamingOptions.class).isStreaming() determines if it is batch or streaming. Updated comments to show this more clearly

O I see. options.as(StreamingOptions.class).isStreaming() is used at line 530.

amaliujia

Enabled cache can also pass internal test suite.

amaliujia · 2021-08-05T19:40:58Z

Run Java_Examples_Dataflow_Java11 PreCommit

amaliujia · 2021-08-05T19:41:12Z

Run Java_Examples_Dataflow PreCommit

anthonyqzhu · 2021-08-05T20:48:15Z

Run Java PreCommit

anthonyqzhu · 2021-08-05T21:34:57Z

Run Java PreCommit

anthonyqzhu · 2021-08-05T22:04:25Z

Run Java PreCommit

amaliujia · 2021-08-06T17:03:19Z

The failing org.apache.beam.examples.WordCountIT.testE2EWordCount is a known issue (and there is a ongoing fix). Per plat mon's suggestion we can merge PR in this case.

lukecwik · 2021-08-11T16:51:26Z

sdks/java/harness/src/main/java/org/apache/beam/fn/harness/control/ProcessBundleHandler.java

+    // Instantiate a State API call handler depending on whether a State ApiServiceDescriptor was
+    // specified.
+    HandleStateCallsForBundle beamFnStateClient;
+    if (bundleDescriptor.hasStateApiServiceDescriptor()) {


@amaliujia @anthonyqzhu
I was under the impression that we would place the cache behind an experiment instead of opting everyone into its usage.

We merged this assuming it was okay after seeing the passing tests / TAP presubmit passing as well.

I can introduce an experiment in front of this block today if necessary

Agreed. Let's file a patch to hide the cache by an experiment.

anthonyqzhu added 3 commits July 26, 2021 18:54

[BEAM-10212] Add state cache to ProcessBundleHandler

20b54e7

Add testing of iterable side input + user state cross bundle cache

5b304c2

Rename loader and add comment

5184b06

anthonyqzhu force-pushed the BEAM-10212-integrate-caching-client branch from 0669dea to 5184b06 Compare July 26, 2021 19:07

anthonyqzhu changed the title ~~[Beam-10212] Integrate caching client [WIP]~~ [Beam-10212] Integrate caching client Jul 26, 2021

Test clear caching across bundle

a42362b

amaliujia changed the title ~~[Beam-10212] Integrate caching client~~ [BEAM-10212] Integrate caching client Jul 30, 2021

anthonyqzhu added 2 commits August 5, 2021 07:21

Only enable batch for now due to streaming bug

46ee38f

Cleanup and add comments

6c7f798

anthonyqzhu force-pushed the BEAM-10212-integrate-caching-client branch from 460fac3 to 6c7f798 Compare August 5, 2021 15:14

amaliujia reviewed Aug 5, 2021

View reviewed changes

Split up comments better

170a7a6

amaliujia approved these changes Aug 5, 2021

View reviewed changes

amaliujia merged commit 9c9903d into apache:master Aug 6, 2021

lukecwik reviewed Aug 11, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-10212] Integrate caching client #15214

[BEAM-10212] Integrate caching client #15214

anthonyqzhu commented Jul 23, 2021

anthonyqzhu commented Jul 23, 2021

codecov bot commented Jul 26, 2021 •

edited

Loading

amaliujia commented Jul 27, 2021

amaliujia commented Aug 5, 2021

amaliujia commented Aug 5, 2021

amaliujia Aug 5, 2021

anthonyqzhu Aug 5, 2021

amaliujia Aug 5, 2021

amaliujia left a comment

amaliujia commented Aug 5, 2021

amaliujia commented Aug 5, 2021

anthonyqzhu commented Aug 5, 2021

anthonyqzhu commented Aug 5, 2021

anthonyqzhu commented Aug 5, 2021

amaliujia commented Aug 6, 2021

lukecwik Aug 11, 2021

anthonyqzhu Aug 11, 2021

lukecwik Aug 11, 2021

amaliujia Aug 11, 2021 •

edited

Loading

[BEAM-10212] Integrate caching client #15214

[BEAM-10212] Integrate caching client #15214

Conversation

anthonyqzhu commented Jul 23, 2021

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)

anthonyqzhu commented Jul 23, 2021

codecov bot commented Jul 26, 2021 • edited Loading

Codecov Report

amaliujia commented Jul 27, 2021

amaliujia commented Aug 5, 2021

amaliujia commented Aug 5, 2021

amaliujia Aug 5, 2021

Choose a reason for hiding this comment

anthonyqzhu Aug 5, 2021

Choose a reason for hiding this comment

amaliujia Aug 5, 2021

Choose a reason for hiding this comment

amaliujia left a comment

Choose a reason for hiding this comment

amaliujia commented Aug 5, 2021

amaliujia commented Aug 5, 2021

anthonyqzhu commented Aug 5, 2021

anthonyqzhu commented Aug 5, 2021

anthonyqzhu commented Aug 5, 2021

amaliujia commented Aug 6, 2021

lukecwik Aug 11, 2021

Choose a reason for hiding this comment

anthonyqzhu Aug 11, 2021

Choose a reason for hiding this comment

lukecwik Aug 11, 2021

Choose a reason for hiding this comment

amaliujia Aug 11, 2021 • edited Loading

Choose a reason for hiding this comment

`ValidatesRunner` compliance status (on master branch)

codecov bot commented Jul 26, 2021 •

edited

Loading

amaliujia Aug 11, 2021 •

edited

Loading