Skip to content

[BEAM-9822] Simplify pipeline when batching is disabled. - #11529

Merged
TheNeuralBit merged 3 commits into
apache:masterfrom
nielm:skipBatchingIfDisabled
May 21, 2020
Merged

[BEAM-9822] Simplify pipeline when batching is disabled.#11529
TheNeuralBit merged 3 commits into
apache:masterfrom
nielm:skipBatchingIfDisabled

Conversation

@nielm

@nielmnielm commented Apr 26, 2020

Copy link
Copy Markdown
Contributor

Batching is often disabled in streaming pipelines to improve latency. When this is the case, there is no
point in adding several transforms to group/sort/batch the elements, so if batching is disabled, a simple pipeline is used.

Note, this PR is dependent on PR #11528 and PR #11532

Post-Commit Tests Status (on master branch)

LangSDKApexDataflowFlinkGearpumpSamzaSpark
GoBuild Status------Build Status------Build Status
JavaBuild StatusBuild StatusBuild Status
Build Status
Build Status
Build Status
Build Status
Build Status
Build StatusBuild StatusBuild Status
Build Status
Build Status
PythonBuild Status
Build Status
Build Status
Build Status
---Build Status
Build Status
Build Status
Build Status
Build Status
------Build Status
XLang---------Build Status------Build Status

Pre-Commit Tests Status (on master branch)

---JavaPythonGoWebsite
Non-portableBuild StatusBuild Status
Build Status
Build StatusBuild Status
Portable---Build Status------

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

@allenpradeepallenpradeep left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@nielm
nielmforce-pushed the skipBatchingIfDisabled branch 2 times, most recently from 2955973 to 718884bCompareMay 18, 2020 23:43
@TheNeuralBit

Copy link
Copy Markdown
Member

Retest this please

nielm added 2 commits May 19, 2020 13:06
Grouping adds significant latency and memory use, and when streaming
this causes both OOMs and high pipeline latencies.
When batching is disabled, there is no need for SpannerIO to read the
schema, group, sort, batch and write batches, so simplify the pipeline
to just write the mutation.
@nielm
nielmforce-pushed the skipBatchingIfDisabled branch from 718884b to ade11d9CompareMay 19, 2020 11:15
@nielm

Copy link
Copy Markdown
ContributorAuthor

Retest this please

@TheNeuralBit

Copy link
Copy Markdown
Member

Retest this please

@TheNeuralBitTheNeuralBit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, just one minor request

@nielm

Copy link
Copy Markdown
ContributorAuthor

Retest this please

1 similar comment
@TheNeuralBit

Copy link
Copy Markdown
Member

Retest this please

@TheNeuralBit

Copy link
Copy Markdown
Member

LGTM. I'll merge when CI is green

@TheNeuralBit

Copy link
Copy Markdown
Member

Retest this please

3 similar comments
@TheNeuralBit

Copy link
Copy Markdown
Member

Retest this please

@TheNeuralBit

Copy link
Copy Markdown
Member

Retest this please

@TheNeuralBit

Copy link
Copy Markdown
Member

Retest this please

@TheNeuralBit

Copy link
Copy Markdown
Member

Jenkins seems to be very sleepy today...

@TheNeuralBit

Copy link
Copy Markdown
Member

Retest this please

@TheNeuralBit

Copy link
Copy Markdown
Member

retest this please

@udim

udim commented May 20, 2020

Copy link
Copy Markdown
Member

Retest this please

1 similar comment
@TheNeuralBit

Copy link
Copy Markdown
Member

Retest this please

@TheNeuralBit

Copy link
Copy Markdown
Member

Run Java PostCommit

1 similar comment
@TheNeuralBit

Copy link
Copy Markdown
Member

Run Java PostCommit

@TheNeuralBit

Copy link
Copy Markdown
Member

Run Java PreCommit

1 similar comment
@TheNeuralBit

Copy link
Copy Markdown
Member

Run Java PreCommit

@TheNeuralBit
TheNeuralBit merged commit b33ed49 into apache:masterMay 21, 2020
yirutang pushed a commit to yirutang/beam that referenced this pull request Jul 23, 2020
* Disable grouping by default when streaming.
Grouping adds significant latency and memory use, and when streaming
this causes both OOMs and high pipeline latencies.
* Simplify pipeline when batching is disabled.
When batching is disabled, there is no need for SpannerIO to read the
schema, group, sort, batch and write batches, so simplify the pipeline
to just write the mutation.
* Fix noBatching test
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@nielm@TheNeuralBit@udim@allenpradeep