You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2021/08/26 17:38:46 UTC
[GitHub] [beam] jrmccluskey opened a new pull request #15400: [BEAM-11928] Implement stand-along Combine Globally operation
jrmccluskey opened a new pull request #15400:
URL: https://github.com/apache/beam/pull/15400
Modify TryCombine to use standard combine_globally URN instead of routing through CombinePerKey. Extends the current method of handling combines to include a special CombineGlobally scope name.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) 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](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
`ValidatesRunner` compliance status (on master branch)
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_ULR/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Streaming/lastCompletedBuild/badge/icon?subject=V1+Streaming">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon?subject=V1+Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_VR_Dataflow_V2_Streaming/lastCompletedBuild/badge/icon?subject=V2+Streaming">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon?subject=Java+8">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon?subject=Java+11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon?subject=Portable+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Samza/lastCompletedBuild/badge/icon?subject=Portable">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon?subject=Structured+Streaming">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon?subject=ValCont">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/badge/icon?subject=Portable">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Samza/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
</tr>
</tbody>
</table>
Examples testing status on various runners
--------------------------------------------------------
<table>
<thead>
<tr>
<th>Lang</th>
<th>ULR</th>
<th>Dataflow</th>
<th>Flink</th>
<th>Samza</th>
<th>Spark</th>
<th>Twister2</th>
</tr>
</thead>
<tbody>
<tr>
<td>Go</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Java</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Cron/lastCompletedBuild/badge/icon?subject=V1">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Examples_Dataflow_Java11_Cron/lastCompletedBuild/badge/icon?subject=V1+Java11">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java_Examples_Dataflow_V2/lastCompletedBuild/badge/icon?subject=V2">
</a><br>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>Python</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
<tr>
<td>XLang</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
Post-Commit SDK/Transform Integration Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>Go</th>
<th>Java</th>
<th>Python</th>
</tr>
</thead>
<tbody>
<tr>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon?subject=3.6">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon?subject=3.7">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/badge/icon?subject=3.8">
</a>
</td>
</tr>
</tbody>
</table>
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
<table>
<thead>
<tr>
<th>---</th>
<th>Java</th>
<th>Python</th>
<th>Go</th>
<th>Website</th>
<th>Whitespace</th>
<th>Typescript</th>
</tr>
</thead>
<tbody>
<tr>
<td>Non-portable</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon">
</a><br>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon?subject=Tests">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon?subject=Lint">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/badge/icon?subject=Docker">
</a><br>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/badge/icon?subject=Docs">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
</tr>
<tr>
<td>Portable</td>
<td>---</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>
<a href="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/">
<img alt="Build Status" src="https://ci-beam.apache.org/job/beam_PreCommit_GoPortable_Cron/lastCompletedBuild/badge/icon">
</a>
</td>
<td>---</td>
<td>---</td>
<td>---</td>
</tr>
</tbody>
</table>
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-919325116
Run Go Spark ValidatesRunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-920320325
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-910733712
Run Go PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-924335903
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-along Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-910492760
Run Go Flink ValidatesRunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-914518273
Yeah it seems to be that hard-coded expectation of a stream. That's what gave rise to the direct runner having to drop in a CoGBK ahead of the global combine, since the CoGBK outputs in that FullValue/ReStream pair. I'm a little surprised the Flink runner doesn't like it given that every other runner does, though. And adding logic to handle the nil ReStream case will likely not fix it, as that means it is outputting individual elements to process element instead of arranging them to be processed together. The next operation would only get the accumulation function output for a single element, not for all of the elements.
Regardless, it makes sense that the expectation of getting a ReStream should be wrapped with some checking and a more helpful error message than just a generic index out of bounds panic. I added a quick check and we can go from there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-925410954
Yes the failures are with the TestStream/trigger tests. I modified the only active teststream test to handle the altered behavior to make sure that was the only issue with the Flink test suite, so everything is green apart from the discussed issues
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] youngoli commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
youngoli commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-923208237
Gotcha, it makes sense then that it would be the same issue with that TestStream test.
In my PR I ran into this with an XLang test (TestXLang_Multi), so it looks like the environment setup cause problems even without TestStream. Probably something we should try and fix soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-920320517
Run Go Samza ValidatesRunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-916186576
Run Go Flink ValidatesRunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15400:
URL: https://github.com/apache/beam/pull/15400#discussion_r705808589
##########
File path: sdks/go/pkg/beam/core/runtime/exec/translate.go
##########
@@ -457,7 +457,7 @@ func (b *builder) makeLink(from string, id linkID) (Node, error) {
}
}
- case graph.Combine:
+ case graph.Combine, graph.CombineGlobally:
cn := &Combine{UID: b.idgen.New(), Out: out[0]}
cn.Fn, err = graph.AsCombineFn(fn)
if err != nil {
Review comment:
We should print out, in the urn switch below, what urn we're not handling in the default case. `default: // For unlifted combines`
We shouldn't be missing any urns really, but a paranoid check can't hurt.
The [doc for lifted combines](https://docs.google.com/document/d/1-3mEs3Y7bIkJ0hmQ6SiHpVIFu5vbY6Zcpw-7tOMVg4U/edit#heading=h.i62jhvh3wkod) doesn't mention any different handling for global combines though, which is frustrating, and the proto only mentions we should understand the combine components, not that there's a "accumulate things locally" option that doesn't have an iterable value...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-923200852
It first came up in the initial TestStream implementation, it's something with how Flink is setting up its environment. We asked around when TestStream was first submitted and no one seemed to know, which is why those tests were generally not enabled. Seems to be machine specific, impacting places like Jenkins but passing on other machines without issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-914452850
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-along Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-910485294
Run GoPortable PreCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-along Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-906641233
Run Go PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-920320247
Run Go PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-914430183
Run Go Flink ValidatesRunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
lostluck commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-925405490
Looks like that last change seems to have resolved the Flink issues (modulo anything that's going through test stream). Offline you mentioned failures, are they only happening for the disabled trigger and teststream tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-924286983
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-919325318
Run Go Samza ValidatesRunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-932439176
Solution for global combines being handled in different approach, closing PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-920990899
R: @youngoli @riteshghorse
The Flink failure is the same TestStream memory buffer error we saw before, will likely have to re-write the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-920357653
Run Go Flink ValidatesRunner
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15400:
URL: https://github.com/apache/beam/pull/15400#discussion_r715909229
##########
File path: sdks/go/pkg/beam/core/graph/edge.go
##########
@@ -450,8 +456,8 @@ func NewCombine(g *Graph, s *Scope, u *CombineFn, in *Node, ac *coder.Coder, typ
}
inT := in.Type()
- if !typex.IsCoGBK(inT) {
- return nil, addContext(errors.Errorf("Combine requires CoGBK type: %v", inT), s)
+ if !typex.IsCoGBK(inT) && s.Label == CombinePerKeyScope {
Review comment:
With the above changes, this change here is unnecessary since the GBK type will be valid.
##########
File path: sdks/go/pkg/beam/core/runtime/exec/combine.go
##########
@@ -143,6 +143,10 @@ func (n *Combine) ProcessElement(ctx context.Context, value *FullValue, values .
}
first := true
+ if len(values) == 0 {
+ return errors.Errorf("did not get values to combine for key %v, ID %v", value.Elm, n.UID)
Review comment:
The lack of pre-GBK in the fallback case is why we ended up with this error at all.
##########
File path: sdks/go/pkg/beam/combine.go
##########
@@ -39,12 +39,8 @@ func CombinePerKey(s Scope, combinefn interface{}, col PCollection, opts ...Opti
// for multiple reasons, notably that the combinefn is not valid or cannot be bound
// -- due to type mismatch, say -- to the incoming PCollections.
func TryCombine(s Scope, combinefn interface{}, col PCollection, opts ...Option) (PCollection, error) {
- pre := AddFixedKey(s, col)
- post, err := TryCombinePerKey(s, combinefn, pre, opts...)
- if err != nil {
- return PCollection{}, err
- }
- return DropKey(s, post), nil
+ s = s.Scope(graph.CombineGloballyScope)
Review comment:
I think I understand the other bugs that have been happening when a runner doesn't understand the combine_globally urn. And now I feel like I've wasted your time for a few weeks. Very sorry about that :/.
This change is removing the fallback behaviour from the pipeline graph for unknown composites. The intent of that is to build up sub transforms that do the same work as the URN that is implementing them.
The existing code for beam.Combine added a fixed key, then deferred to CombinePerKey, which meant that downstream in graphx/translate.go it lost any sense of the "global" nature of this particular transform. But also, I don't think I've explained how CombinePerKey as a composite transform works. If you read the original combine per key code, note that it always adds a TryGroupByKey, and follows it by a NewCombine.
So in the case of a GlobalCombine, what we want is to
1. Add the "magic scope" (`graph.CombineGloballyScope`).
2. AddFixedKey + TryGroupByKey + NewCombine + DropKey
3. In graphx, we check and detect the magic scope and conditions (similarly to how we're doing it for CombinePerKey).
It's very important that we have a composite 2 with all those contents, since that's what needs to get executed by the runner if it doesn't understand the URN at all. "NewCombine" by itself doesn't actually do any grouping.
Basically, in principal, there shouldn't be any need for the Direct runner to change, (with new nodes or anything) since it's the kind of runner that will rely on the fallbacks.
##########
File path: sdks/go/test/integration/primitives/teststream.go
##########
@@ -21,17 +21,18 @@ import (
"github.com/apache/beam/sdks/v2/go/pkg/beam/testing/teststream"
)
Review comment:
The lack of built in / fallback GBK is probably why the Trigger tests started failing hard: no point where a GBK as happening anymore, which is when triggers occur.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15400:
URL: https://github.com/apache/beam/pull/15400#discussion_r703888175
##########
File path: sdks/go/pkg/beam/core/runtime/graphx/translate.go
##########
@@ -251,11 +252,11 @@ func (m *marshaller) addScopeTree(s *ScopeTree) (string, error) {
// Beam Portability requires that composites contain an implementation for runners
// that don't understand the URN and Payload, which this lightly checks for.
func (m *marshaller) updateIfCombineComposite(s *ScopeTree, transform *pipepb.PTransform) error {
Review comment:
It would likely go in a if block in the default case for " For unlifted combines" when the combine is a global one, in exec/translate.go.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-910560776
Run Go PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-910562545
R: @lostluck
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15400:
URL: https://github.com/apache/beam/pull/15400#discussion_r703887294
##########
File path: sdks/go/pkg/beam/core/runtime/graphx/translate.go
##########
@@ -251,11 +252,11 @@ func (m *marshaller) addScopeTree(s *ScopeTree) (string, error) {
// Beam Portability requires that composites contain an implementation for runners
// that don't understand the URN and Payload, which this lightly checks for.
func (m *marshaller) updateIfCombineComposite(s *ScopeTree, transform *pipepb.PTransform) error {
Review comment:
I think the problem with Flink here is that it doesn't understand how to handle URNCombineGlobally. As the doc comment for this function says, "// Beam Portability requires that composites contain an implementation for runners
// that don't understand the URN and Payload".
Flink then falls back on whatever implementation is in the subtransforms (see lines 228-240), which simply have "GBK + Combine".
In essence, it could be that the exec.Combine node is wrong for this case, as it would only be used for this unlifted case. It was fine before, since fixed key handling changed what the Flink GBK was receiving.
So one solution is: change it so the subtransforms are updated with the fixed key logic somehow, working something like the special Reshuffle or CoGBK handler nodes work. Probably more work that way.
An alternative would be to add a new/different CombineGlobally node to the exec package that can handle serial elements (likely doing something with AddInput and handing it to Extract output, when it's a CombineGlobally graph node). This is probably less work than the former option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] youngoli commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
youngoli commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-923193753
I'm still reviewing the code, but I'll comment on the error first. I'm assuming this is the main error:
```
13:41:31 2021/09/15 20:41:31 (): java.io.IOException: Insufficient number of network buffers: required 17, but only 14 available. The total number of network buffers is currently set to 2048 of 32768 bytes each. You can increase this number by setting the configuration keys 'taskmanager.memory.network.fraction', 'taskmanager.memory.network.min', and 'taskmanager.memory.network.max'.
```
I started running into this error on a PR of my own, where I'm adding Go to the x-lang test suites. I'm not sure the exact reason started popping up, but it definitely looks like our tests are using too many network resources for some reason, although I find that strange because AFAIK we should have no parallelism while running the Flink tests.
We should probably find someone who might already know what's going on with this error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey closed pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey closed pull request #15400:
URL: https://github.com/apache/beam/pull/15400
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-919324723
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lostluck commented on a change in pull request #15400: [BEAM-11928] Implement stand-alone Combine Globally operation
Posted by GitBox <gi...@apache.org>.
lostluck commented on a change in pull request #15400:
URL: https://github.com/apache/beam/pull/15400#discussion_r703651332
##########
File path: sdks/go/pkg/beam/core/graph/edge.go
##########
@@ -450,8 +456,8 @@ func NewCombine(g *Graph, s *Scope, u *CombineFn, in *Node, ac *coder.Coder, typ
}
inT := in.Type()
- if !typex.IsCoGBK(inT) {
- return nil, addContext(errors.Errorf("Combine requires CoGBK type: %v", inT), s)
+ if !typex.IsCoGBK(inT) && s.Label == CombinePerKeyScope {
+ return nil, addContext(errors.Errorf("Combine Per Krequires CoGBK type: %v", inT), s)
Review comment:
typo. Also, unless it helps reading, keeping the error wording to match terms the user writes helps debugging. Errors should be written in terms the user who runs into them can understand and handle. In this case, no spaces for "CombinePerKey".
```suggestion
return nil, addContext(errors.Errorf("CombinePerKey requires CoGBK type: %v", inT), s)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] jrmccluskey commented on pull request #15400: [BEAM-11928] Implement stand-along Combine Globally operation
Posted by GitBox <gi...@apache.org>.
jrmccluskey commented on pull request #15400:
URL: https://github.com/apache/beam/pull/15400#issuecomment-910424373
Run Go PostCommit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org