[BEAM-11408] Integrate BigQuery sink streaming inserts with GroupIntoBatches by nehsyc · Pull Request #13496 · apache/beam

nehsyc · 2020-12-07T19:11:26Z

Use GroupIntoBatches.WithShardedKey API to group and batch write before streaming to BigQuery service. Currently batching is done best-effort on bundle finalization.

This PR

adds an option to BigQueryOptions to toggle between the existing and new implementation;
extracts the shared code between the old and new implementation to a class BatchedStreamingWrite and provides an option to choose the implementation.

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.

Post-Commit Tests Status (on master branch)

Lang	Dataflow	Samza	Twister2
Go	---	---	---
Java
Python		---	---
XLang		---	---

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.

nehsyc · 2020-12-07T21:48:21Z

R: @reuvenlax @chamikaramj

nehsyc · 2020-12-28T18:58:58Z

Not sure about this limit. What would be a proper value? Should we make it configurable?

to be hones, I am not sure what's a good duration either. I think this is acceptable for now, until we find out more. Thoughts?

Yeah sounds reasonable to proceed with this for now.

reuvenlax · 2021-01-06T06:42:19Z

We are relying on the fact that the GroupIntoBatches produces stable output. Really we should tag this with RequiresStableInput. Can you find out if this is safe to do in Dataflow?

There is a transform override in Dataflow to add a preceding Reshuffle to DoFns marked with RequiresStableInput.
beam/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/RequiresStableInputParDoOverrides.java
Line 73 in 74ec609
.apply("Materialize input", Reshuffle.viaRandomKey())
The override is disabled though. So I guess currently Dataflow does nothing for this tag.
beam/runners/google-cloud-dataflow-java/src/main/java/org/apache/beam/runners/dataflow/DataflowRunner.java
Line 601 in df74d74
/* TODO[Beam-4684]: Support @RequiresStableInput on Dataflow in a more intelligent way
Also my understanding is that dding a Reshuffle before GroupIntoBatches will introduce an extra shuffle as Reshuffle is essentially a GBK + value expansion in Dataflow.

nehsyc · 2021-01-15T21:09:01Z

@reuvenlax I also have changes for FILE_LOADS ready in my local branch. If it is ok for you, I can merge those into this PR. Otherwise I will send a follow-up PR.

nehsyc · 2021-01-26T00:50:54Z

R: @pabloem

nehsyc · 2021-01-26T00:55:55Z

R: @pabloem

Hey Pablo, let me know if it is ok for you to include the changes in FILE_LOADS in this PR as well. If so, I will push a new commit.

pabloem · 2021-01-27T20:49:06Z

Run Java PostCommit

pabloem · 2021-01-27T20:49:15Z

Run Java PreCommit

pabloem · 2021-01-27T22:18:10Z

There are a couple checkstyle warnings on Precommit:

[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_Phrase/src/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BatchedStreamingWrite.java:85:24: Name 'LOG' must match pattern '^[a-z][a-zA-Z0-9]*$'. [MemberName] | | [ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_Phrase/src/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BatchedStreamingWrite.java:279:28: Name 'BATCH_MAX_BUFFERING_DURATION' must match pattern '^[a-z][a-zA-Z0-9]*$'. [MemberName]

These mean that the variables should be static to have CAPITALIZED_NAMES, or should be named with camelCase if they are not static. Can you fix that?

Also, it seems there are some merge conflicts. Can you fix that as well?

The last thing to figure out is how to address Reuven's comment regarding stable input

…ddressed comments.

pabloem · 2021-01-27T22:35:16Z

Run Java PostCommit

pabloem · 2021-01-27T22:43:31Z

Run Java_Examples_Dataflow_Java11 PreCommit

nehsyc · 2021-01-27T22:46:23Z

There are a couple checkstyle warnings on Precommit:

[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_Phrase/src/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BatchedStreamingWrite.java:85:24: Name 'LOG' must match pattern '^[a-z][a-zA-Z0-9]*$'. [MemberName] | | [ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_Phrase/src/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BatchedStreamingWrite.java:279:28: Name 'BATCH_MAX_BUFFERING_DURATION' must match pattern '^[a-z][a-zA-Z0-9]*$'. [MemberName]

These mean that the variables should be static to have CAPITALIZED_NAMES, or should be named with camelCase if they are not static. Can you fix that?

Thanks for pointing out! Pushed a commit to fix those.

Also, it seems there are some merge conflicts. Can you fix that as well?

Done.

The last thing to figure out is how to address Reuven's comment regarding stable input

Based on my understanding Dataflow currently doesn't do anything for @RequiresStableInputs so it may be considered as safe for now but if we add naive support like a Reshuffle it would be adding duplicated shuffles. How about adding a TODO here so we don't forget?

pabloem · 2021-01-27T22:48:30Z

There seems to be an issue with a try/catch: https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Commit/12361/console

pabloem · 2021-01-28T00:17:59Z

Run Java PostCommit

pabloem · 2021-01-28T01:59:00Z

Run Java PreCommit

pabloem · 2021-01-28T01:59:16Z

Postcommit from previous commit: https://ci-beam.apache.org/job/beam_PostCommit_Java_PR/558/

pabloem · 2021-01-28T03:18:41Z

Thanks @nehsyc !

…sink streaming inserts with GroupIntoBatches * Integrate BQ streaming inserts with GroupIntoBatches * Moved autosharding option from BigQueryOption to BigQueryIOBuilder; addressed comments. * fix checkstyle error * Revert the logic that was dropped during merge * Add comments for RequiresStableInput

probot-autolabelerBot added gcp io java labels Dec 7, 2020

nehsyc force-pushed the bq-integration-java branch from 468b4b7 to d187951Compare December 7, 2020 21:07

nehsyc commented Dec 7, 2020
View reviewed changes

Comment thread...-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BatchedStreamingWrite.java Outdated

aaltay requested review from chamikaramj and reuvenlax December 18, 2020 02:23

nehsyc force-pushed the bq-integration-java branch from d187951 to 3b0992cCompare December 28, 2020 18:57

nehsyc commented Dec 28, 2020
View reviewed changes

reuvenlax reviewed Jan 6, 2021
View reviewed changes

nehsyc force-pushed the bq-integration-java branch 2 times, most recently from f8790e3 to 0ab05d9Compare January 9, 2021 00:52

nehsyc requested a review from reuvenlax January 11, 2021 17:37

nehsyc added 3 commits January 27, 2021 14:27

Integrate BQ streaming inserts with GroupIntoBatches
b16f518

Moved autosharding option from BigQueryOption to BigQueryIOBuilder; a…
129176a
…ddressed comments.

fix checkstyle error
c033215

nehsyc force-pushed the bq-integration-java branch from 0ab05d9 to c033215Compare January 27, 2021 22:31

Revert the logic that was dropped during merge
3deba1d

Add comments for RequiresStableInput
1b97e7c

pabloem merged commit f0a3f6d into apache:masterJan 28, 2021

This was referenced Jan 28, 2021
[BEAM-11408] [cherry-pick] Integrate BigQuery sink streaming inserts with GroupIntoBatches #13832
Merged
[BEAM-11772] Integrate BigQuery sink file loads with GroupIntoBatches #13859
Merged

nehsyc mentioned this pull request Feb 22, 2021
[BEAM-11408, BEAM-11772] Integrate Python BigQuery sink with GroupIntoBatches #14033
Merged
4 tasks

Uh oh!

Conversation

nehsyc commented Dec 7, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

GitHub Actions Tests Status (on master branch)

Uh oh!

nehsyc commented Dec 7, 2020

Uh oh!

Uh oh!

nehsycDec 28, 2020

Choose a reason for hiding this comment

Uh oh!

pabloemJan 27, 2021

Choose a reason for hiding this comment

Uh oh!

nehsycJan 27, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

reuvenlaxJan 6, 2021

Choose a reason for hiding this comment

Uh oh!

nehsycJan 8, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nehsyc commented Jan 15, 2021

Uh oh!

nehsyc commented Jan 26, 2021

Uh oh!

nehsyc commented Jan 26, 2021

Uh oh!

pabloem commented Jan 27, 2021

Uh oh!

pabloem commented Jan 27, 2021

Uh oh!

pabloem commented Jan 27, 2021

Uh oh!

pabloem commented Jan 27, 2021

Uh oh!

pabloem commented Jan 27, 2021

Uh oh!

nehsyc commented Jan 27, 2021

Uh oh!

pabloem commented Jan 27, 2021

Uh oh!

pabloem commented Jan 28, 2021

Uh oh!

pabloem commented Jan 28, 2021

Uh oh!

pabloem commented Jan 28, 2021

Uh oh!

pabloem commented Jan 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nehsyc commented Dec 7, 2020•
edited
Loading