[BEAM-12260] Java - Backport Firestore connector's ramp-up throttling to Datastore connector #14713

danthev · 2021-05-03T19:34:33Z

A new connector for Cloud Firestore is being added to Beam and currently out for review (see #14261). Some of the features and adjustments are useful for Cloud Datastore as well, so they're being backported to this connector.

Ramp-up throttling

This follows the same implementation as the Firetore connector, though implemented as a subtransform in the Write/Delete PTransform for simplicity and so the change is more easily visible.
In essence, the ramp-up throttler is a generic rate limiter that emits elements only when there is sufficient "budget". This budget starts at 500 entities per second (split across workers) and increases by 50% every 5 minutes, as documented under Datastore Best Practices. Abiding by this rule is not always necessary, the throttling step can be turned off in that case.

Minor tweaks

There have been minor tweaks and adjustments to some of the constants along the way.

Raised target latency for automatic batch sizing from 5 to 6 seconds.
Decreased initial batch size from 200 to 50.
Decresed minimum batch size from 10 to 5.
The default user agent from pipeline options is set, so the Beam version used is visible by the Datastore team for debugging.
Add two metrics to help users debug, datastoreEntitiesMutated and datastoreLatencyMsPerMutation.

R: @chamikaramj

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	Samza	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.

…ling

…or-java/clientside-throttling

aaltay · 2021-05-13T22:58:44Z

@chamikaramj - feel free to delegate to someone else.

@danthev - thank you!

chamikaramj · 2021-05-18T18:10:30Z

Run Java PreCommit

chamikaramj · 2021-05-21T00:22:53Z

Same questions as #14723 but we can do the conversation there.

chamikaramj · 2021-05-21T00:23:58Z

R: @nehsyc

…or-java/clientside-throttling

chamikaramj · 2021-06-07T05:29:24Z

R: @udim

(also please note the corresponding Python changes and the current review from @nehsyc in #14723)

…or-java/clientside-throttling

danthev · 2021-06-11T23:16:21Z

Run Java PreCommit

danthev · 2021-06-12T00:07:04Z

There seems to be a broken Flink test in master, otherwise this is ready for review.

…or-java/clientside-throttling

danthev · 2021-06-15T16:56:50Z

Tests are green, so change is ready for review.

nehsyc

Thanks Daniel! Generally looks good :)

...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1.java

...le-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/RampupThrottlingFn.java

…or-java/clientside-throttling

nehsyc

Thanks :)

...le-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/RampupThrottlingFn.java

nehsyc · 2021-06-17T06:15:12Z

...loud-platform/src/test/java/org/apache/beam/sdk/io/gcp/datastore/RampupThrottlingFnTest.java

+
+/** Tests for {@link RampupThrottlingFn}. */
+@RunWith(JUnit4.class)
+public class RampupThrottlingFnTest {


Any way to test throttling counter is set properly?

Done. (Had to make throttlingMsecs visible for testing)

…or-java/clientside-throttling

danthev · 2021-06-17T19:18:12Z

Run Apache Beam Java SDK Quickstart - Direct

danthev · 2021-06-17T19:19:57Z

retest this please

danthev · 2021-06-17T19:20:38Z

Java Wordcount failure seems unrelated, but I can't get it to rerun the test.

…or-java/clientside-throttling

danthev · 2021-06-18T20:33:30Z

Run Java PreCommit

chamikaramj

Thanks. LGTM other than a nit.

chamikaramj · 2021-06-18T20:47:39Z

...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1.java

@@ -212,7 +220,7 @@
   * <p>Testing has found that a batch of 200 entities will generally finish within the timeout even
   * in adverse conditions.
   */
-  @VisibleForTesting static final int DATASTORE_BATCH_UPDATE_ENTITIES_START = 200;
+  @VisibleForTesting static final int DATASTORE_BATCH_UPDATE_ENTITIES_START = 50;


Why was this updated ? If 50 is a better number could you also update the comment please.

I've updated the comment. We found that a lower starting value helps with entities that have an extremely high number of indexes, these would sometimes start with load that had no realistic chance of succeeding. This is just a start value, so the size quickly corrects up to 500 if write throughput is enough after all.

chamikaramj · 2021-06-18T20:47:58Z

...io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1.java

@@ -225,7 +233,7 @@
   * number of entities per request may be lower when we flush for the end of a bundle or if we hit
   * {@link DatastoreV1.DATASTORE_BATCH_UPDATE_BYTES_LIMIT}.
   */
-  @VisibleForTesting static final int DATASTORE_BATCH_UPDATE_ENTITIES_MIN = 10;
+  @VisibleForTesting static final int DATASTORE_BATCH_UPDATE_ENTITIES_MIN = 5;


Ditto. Note that this could potentially double the number of RPCs to Datastore.

Somewhat similar issue, the limit of 10 was rather arbitrary and led to some deadlines on expensive writes. Write cost outweighs the cost of processing more RPCs, so we went down a little to accommodate that.
Both variables help with edge cases on complex entities, most traffic will be able to go to batch size 500.

chamikaramj

Thanks for the clarifications.

danthev · 2021-06-18T23:06:25Z

Thanks! Rerunning precommit, there's a FlinkSavePoint test that I've noticed more than once now, probably flaky.

Run Java PreCommit

danthev · 2021-06-18T23:06:44Z

Run Java PreCommit

tvalentyn · 2021-09-08T23:39:01Z

It looks like the test added in this change is flaky: https://issues.apache.org/jira/browse/BEAM-12858. @danthev would you by chance be able to take a look?

danthev added 22 commits March 19, 2021 18:58

Initial implementation of the ramp-up throttler.

112d1ce

Merge branch 'master' into datastore-connector-java/clientside-thrott…

36739f9

…ling

Fix unit tests.

a1a4787

Implemented resharding ramp-up

0cd7738

Fix nullable field to make tests run.

396124b

Add throttler shards parameter, some fixes to ramp-up throttler

ee93491

Change rampup budget growth computation

e1b4d95

Restructure throttler loop

cd587a0

Added additional integration tests for batch sizing

67c18e6

Update batch sizing integration tests

302f506

Set user agent, update DatastoreV1 tests.

b33901d

Switch back to hinted worker count, write ramp-up unit test.

63cade3

Make sleeper visible for testing.

ad4cb20

Merge remote-tracking branch 'upstream/master' into datastore-connect…

38d562a

…or-java/clientside-throttling

Fix tests, update documentation.

990aae8

Revert test code

e8fbb62

Merge changes from updated master

0aaabfa

Reformat, fix imports.

ad2bf58

Merge remote-tracking branch 'upstream/master' into datastore-connect…

cda24d5

…or-java/clientside-throttling

Update CHANGES.md

29d2d85

Set minimum ops budget.

8830287

Drop one metric

b46ff1c

danthev changed the title ~~[BEAM-12260] Backport Firestore connector's ramp-up throttling to Datastore connector~~ [BEAM-12260] Java - Backport Firestore connector's ramp-up throttling to Datastore connector May 4, 2021

aaltay requested a review from chamikaramj May 13, 2021 22:58

Merge CHANGES.md

08b2db9

Merge remote-tracking branch 'upstream/master' into datastore-connect…

1073dff

…or-java/clientside-throttling

chamikaramj requested a review from udim June 7, 2021 05:28

Merge remote-tracking branch 'upstream/master' into datastore-connect…

962166a

…or-java/clientside-throttling

Merge remote-tracking branch 'upstream/master' into datastore-connect…

4b3982d

…or-java/clientside-throttling

nehsyc reviewed Jun 16, 2021

View reviewed changes

danthev added 3 commits June 16, 2021 17:03

Update documentation, add explicit throttling budget max value.

5072afd

Remove firstInstant reset from DoFn setup

2ec9b26

Merge remote-tracking branch 'upstream/master' into datastore-connect…

1c51b9f

…or-java/clientside-throttling

nehsyc approved these changes Jun 17, 2021

View reviewed changes

danthev added 5 commits June 17, 2021 11:43

Update comment, verify counter in unit test.

41a9e1f

Merge remote-tracking branch 'upstream/master' into datastore-connect…

eb52390

…or-java/clientside-throttling

Fix format

31b5124

Add newline

bf9864d

Remove spaces

748c485

Merge remote-tracking branch 'upstream/master' into datastore-connect…

08d7135

…or-java/clientside-throttling

chamikaramj reviewed Jun 18, 2021

View reviewed changes

Update comment.

78565af

chamikaramj approved these changes Jun 18, 2021

View reviewed changes

chamikaramj merged commit fcd3fcc into apache:master Jun 21, 2021

Abacn mentioned this pull request Feb 14, 2025

[Bug]: Datastore/Firestore Bulk Delete template performance degration GoogleCloudPlatform/DataflowTemplates#2195

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BEAM-12260] Java - Backport Firestore connector's ramp-up throttling to Datastore connector #14713

[BEAM-12260] Java - Backport Firestore connector's ramp-up throttling to Datastore connector #14713

danthev commented May 3, 2021

aaltay commented May 13, 2021

chamikaramj commented May 18, 2021

chamikaramj commented May 21, 2021

chamikaramj commented May 21, 2021

chamikaramj commented Jun 7, 2021

danthev commented Jun 11, 2021

danthev commented Jun 12, 2021

danthev commented Jun 15, 2021

nehsyc left a comment

nehsyc left a comment

nehsyc Jun 17, 2021

danthev Jun 17, 2021

danthev commented Jun 17, 2021

danthev commented Jun 17, 2021

danthev commented Jun 17, 2021

danthev commented Jun 18, 2021

chamikaramj left a comment

chamikaramj Jun 18, 2021

danthev Jun 18, 2021

chamikaramj Jun 18, 2021

danthev Jun 18, 2021 •

edited

Loading

chamikaramj left a comment

danthev commented Jun 18, 2021

danthev commented Jun 18, 2021

tvalentyn commented Sep 8, 2021

[BEAM-12260] Java - Backport Firestore connector's ramp-up throttling to Datastore connector #14713

[BEAM-12260] Java - Backport Firestore connector's ramp-up throttling to Datastore connector #14713

Conversation

danthev commented May 3, 2021

Ramp-up throttling

Minor tweaks

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)

aaltay commented May 13, 2021

chamikaramj commented May 18, 2021

chamikaramj commented May 21, 2021

chamikaramj commented May 21, 2021

chamikaramj commented Jun 7, 2021

danthev commented Jun 11, 2021

danthev commented Jun 12, 2021

danthev commented Jun 15, 2021

nehsyc left a comment

Choose a reason for hiding this comment

nehsyc left a comment

Choose a reason for hiding this comment

nehsyc Jun 17, 2021

Choose a reason for hiding this comment

danthev Jun 17, 2021

Choose a reason for hiding this comment

danthev commented Jun 17, 2021

danthev commented Jun 17, 2021

danthev commented Jun 17, 2021

danthev commented Jun 18, 2021

chamikaramj left a comment

Choose a reason for hiding this comment

chamikaramj Jun 18, 2021

Choose a reason for hiding this comment

danthev Jun 18, 2021

Choose a reason for hiding this comment

chamikaramj Jun 18, 2021

Choose a reason for hiding this comment

danthev Jun 18, 2021 • edited Loading

Choose a reason for hiding this comment

chamikaramj left a comment

Choose a reason for hiding this comment

danthev commented Jun 18, 2021

danthev commented Jun 18, 2021

tvalentyn commented Sep 8, 2021

`ValidatesRunner` compliance status (on master branch)

danthev Jun 18, 2021 •

edited

Loading