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 2020/09/29 20:17:10 UTC
[GitHub] [beam] guangstick opened a new pull request #12968: [BEAM-7195] Fix BQ BatchLoad not creating new tables issue
guangstick opened a new pull request #12968:
URL: https://github.com/apache/beam/pull/12968
In WriteTables and WriteRename, the `createDisposition` is set to `CREATE_NEVER` when the pane index is great than 0.
https://github.com/apache/beam/blob/ecedd3e654352f1b51ab2caae0fd4665403bd0eb/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/WriteTables.java#L205-L210
https://github.com/apache/beam/blob/ecedd3e654352f1b51ab2caae0fd4665403bd0eb/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/WriteRename.java#L145-L148
Before actually triggering BQ load, BatchLoads groups all temporary files by a singleton key.
https://github.com/apache/beam/blob/ecedd3e654352f1b51ab2caae0fd4665403bd0eb/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BatchLoads.java#L308-L313
This can be problematic when using [DynamicDestinations](https://github.com/apache/beam/blob/ecedd3e654352f1b51ab2caae0fd4665403bd0eb/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/DynamicDestinations.java) in a streaming pipeline with triggers. Sometimes users would like to create tables in the middle of the pipeline based on the data or time. Because of the `GroupOntoSingleton`, different BQ destinations are grouped under the same key (always `Void`), therefore the pane index is always incremental and new tables won't be created even though users have specified `CreateDisposition = CREATE_IF_NEEDED`
Proposed Solution: Instead of grouping into a singleton, grouping by Bigquery destinations.
------------------------
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).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Dataflow | Flink | Samza | Spark | Twister2
--- | --- | --- | --- | --- | --- | ---
Go | [](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/) | ---
Java | [](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Twister2/lastCompletedBuild/)
Python | [](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python38/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/) | ---
XLang | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | [](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/) | ---
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website | Whitespace | Typescript
--- | --- | --- | --- | --- | --- | ---
Non-portable | [](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/)<br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocker_Cron/lastCompletedBuild/) <br>[](https://ci-beam.apache.org/job/beam_PreCommit_PythonDocs_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Whitespace_Cron/lastCompletedBuild/) | [](https://ci-beam.apache.org/job/beam_PreCommit_Typescript_Cron/lastCompletedBuild/)
Portable | --- | [](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | --- | --- | ---
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chamikaramj commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-793371074
Run Java 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik closed pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
lukecwik closed pull request #12968:
URL: https://github.com/apache/beam/pull/12968
--
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] chamikaramj commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-788565851
Run Dataflow 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chamikaramj commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-794590139
Run Java 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-708033099
Ping.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] stale[bot] commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-748441218
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-703786942
so are you saying that the Reshuffle in writeTempTables is inheriting the pane index from this singleton GBK? I though that Reshuffle should be generating brand-new pane indices starting from 0.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-712431302
Yes, this will break update. At this point I'm not sure if we have another option here. The old code was based on an incorrect assumption that Reshuffle generates pane indices, when actually Reshuffle simply forwards the pane indices from the previous GBK.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chamikaramj commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-788565903
Run Java 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guangstick edited a comment on pull request #12968: [BEAM-7195] Fix BQ BatchLoad not creating new tables issue
Posted by GitBox <gi...@apache.org>.
guangstick edited a comment on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-700964600
R: @reuvenlax @chamikaramj
Question: will the PTransform renaming cause compatibility issues when users **update** their GCP Dataflow pipelines?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guangstick commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
guangstick commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-708052716
> R: @reuvenlax @chamikaramj
>
> Question: will the PTransform renaming cause compatibility issues when users **update** their GCP Dataflow pipelines?
Thanks for the review @reuvenlax Yes, I can add a unit test for this. Could you also take a look at this question?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-708038240
FYI - I think that this will fix the bug. Can you add a unit test that exposes the prior bug and is fixed now?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guangstick commented on a change in pull request #12968: [BEAM-7195] Fix BQ BatchLoad not creating new tables issue
Posted by GitBox <gi...@apache.org>.
guangstick commented on a change in pull request #12968:
URL: https://github.com/apache/beam/pull/12968#discussion_r497018910
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/WriteBundlesToFiles.java
##########
@@ -74,7 +74,7 @@
* The result of the {@link WriteBundlesToFiles} transform. Corresponds to a single output file,
* and encapsulates the table it is destined to as well as the file byte size.
*/
- static final class Result<DestinationT> implements Serializable {
+ public static final class Result<DestinationT> implements Serializable {
Review comment:
Open to use getter or use @AutoValue here
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] guangstick commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoad not creating new tables issue
Posted by GitBox <gi...@apache.org>.
guangstick commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-700964600
R: @reuvenlax @chamikaramj
Question: will the PTransform renaming cause compatibility issues when users *update* their GCP Dataflow pipelines?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-799934377
I tried to fix these issues in pr/14238
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] lukecwik closed pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
lukecwik closed pull request #12968:
URL: https://github.com/apache/beam/pull/12968
--
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] guangstick commented on a change in pull request #12968: [BEAM-7195] Fix BQ BatchLoad not creating new tables issue
Posted by GitBox <gi...@apache.org>.
guangstick commented on a change in pull request #12968:
URL: https://github.com/apache/beam/pull/12968#discussion_r497018910
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/WriteBundlesToFiles.java
##########
@@ -74,7 +74,7 @@
* The result of the {@link WriteBundlesToFiles} transform. Corresponds to a single output file,
* and encapsulates the table it is destined to as well as the file byte size.
*/
- static final class Result<DestinationT> implements Serializable {
+ public static final class Result<DestinationT> implements Serializable {
Review comment:
Open to use getter or use `@AutoValue` here
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chamikaramj commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-790780976
Run Java 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] chamikaramj commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-788565342
Retest this please
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] reuvenlax commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
reuvenlax commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-799816848
Been looking at this, and I don't think that this PR works in the the temp-table code path. Looking to see if there's an easy way to fix this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] aaltay commented on pull request #12968: [BEAM-7195] Fix BQ BatchLoads not creating new tables issue
Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #12968:
URL: https://github.com/apache/beam/pull/12968#issuecomment-760411638
Do we want to merge this? Should this be closed?
Or do we not want to do this because it will break updates?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org