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/11/18 21:16:15 UTC
[GitHub] [beam] vachan-shetty opened a new pull request #13378: [BEAM-11208] Fix for QUOTA_EXCEEDED failures in BQ Storage streams sp…
vachan-shetty opened a new pull request #13378:
URL: https://github.com/apache/beam/pull/13378
Fix for BigQuery storage streams failing with QUOTA_EXCEEDED errors in split.
------------------------
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_VR_Dataflow_V2/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_Dataflow/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] kmjung commented on a change in pull request #13378: [BEAM-11208] Fix for QUOTA_EXCEEDED failures in BQ Storage streams sp…
Posted by GitBox <gi...@apache.org>.
kmjung commented on a change in pull request #13378:
URL: https://github.com/apache/beam/pull/13378#discussion_r526457024
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java
##########
@@ -178,6 +179,7 @@ private BigQueryStorageStreamReader(
this.parseFn = source.parseFn;
this.storageClient = source.bqServices.getStorageClient(options);
this.tableSchema = fromJsonString(source.jsonTableSchema, TableSchema.class);
+ this.splitPossible = true;
Review comment:
Nit: I would just initialize this in the declaration above (e.g. ```private boolean splitPossible = true;```) rather than in the constructor.
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java
##########
@@ -288,78 +290,85 @@ public synchronized void close() {
return null;
}
- SplitReadStreamRequest splitRequest =
- SplitReadStreamRequest.newBuilder()
- .setName(source.readStream.getName())
- .setFraction((float) fraction)
- .build();
-
- SplitReadStreamResponse splitResponse = storageClient.splitReadStream(splitRequest);
- if (!splitResponse.hasPrimaryStream() || !splitResponse.hasRemainderStream()) {
- // No more splits are possible!
- Metrics.counter(
- BigQueryStorageStreamReader.class,
- "split-at-fraction-calls-failed-due-to-impossible-split-point")
- .inc();
- LOG.info(
- "BigQuery Storage API stream {} cannot be split at {}.",
- source.readStream.getName(),
- fraction);
- return null;
- }
+ if (splitPossible) {
Review comment:
Nit: I would structure this check as an early return (e.g. ```if (!splitPossible) { return null; }```) rather than putting the whole implementation into a new block.
----------------------------------------------------------------
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] vachan-shetty commented on a change in pull request #13378: [BEAM-11208] Fix for QUOTA_EXCEEDED failures in BQ Storage streams sp…
Posted by GitBox <gi...@apache.org>.
vachan-shetty commented on a change in pull request #13378:
URL: https://github.com/apache/beam/pull/13378#discussion_r526480356
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java
##########
@@ -335,13 +341,15 @@ public synchronized void close() {
+ "the left of the split fraction {}.",
source.readStream.getName(),
fraction);
+ splitPossible = false;
return null;
} catch (Exception e) {
Metrics.counter(
BigQueryStorageStreamReader.class,
"split-at-fraction-calls-failed-due-to-other-reasons")
.inc();
LOG.error("BigQuery Storage API stream split failed.", e);
+ splitPossible = false;
Review comment:
Removed.
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java
##########
@@ -335,13 +341,15 @@ public synchronized void close() {
+ "the left of the split fraction {}.",
source.readStream.getName(),
fraction);
+ splitPossible = false;
Review comment:
Removed.
----------------------------------------------------------------
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] kmjung commented on a change in pull request #13378: [BEAM-11208] Fix for QUOTA_EXCEEDED failures in BQ Storage streams sp…
Posted by GitBox <gi...@apache.org>.
kmjung commented on a change in pull request #13378:
URL: https://github.com/apache/beam/pull/13378#discussion_r526467893
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java
##########
@@ -335,13 +341,15 @@ public synchronized void close() {
+ "the left of the split fraction {}.",
source.readStream.getName(),
fraction);
+ splitPossible = false;
return null;
} catch (Exception e) {
Metrics.counter(
BigQueryStorageStreamReader.class,
"split-at-fraction-calls-failed-due-to-other-reasons")
.inc();
LOG.error("BigQuery Storage API stream split failed.", e);
+ splitPossible = false;
Review comment:
Same comment here.
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java
##########
@@ -335,13 +341,15 @@ public synchronized void close() {
+ "the left of the split fraction {}.",
source.readStream.getName(),
fraction);
+ splitPossible = false;
Review comment:
Nit: I don't think this is correct, or required. Once we've reached this point, we know that the splitReadStream call above has in fact succeeded.
----------------------------------------------------------------
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 merged pull request #13378: [BEAM-11208] Fix for QUOTA_EXCEEDED failures in BQ Storage streams sp…
Posted by GitBox <gi...@apache.org>.
chamikaramj merged pull request #13378:
URL: https://github.com/apache/beam/pull/13378
----------------------------------------------------------------
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] kmjung commented on pull request #13378: [BEAM-11208] Fix for QUOTA_EXCEEDED failures in BQ Storage streams sp…
Posted by GitBox <gi...@apache.org>.
kmjung commented on pull request #13378:
URL: https://github.com/apache/beam/pull/13378#issuecomment-730024659
@chamikaramj can you review?
----------------------------------------------------------------
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 #13378: [BEAM-11208] Fix for QUOTA_EXCEEDED failures in BQ Storage streams sp…
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #13378:
URL: https://github.com/apache/beam/pull/13378#issuecomment-742800978
----------------------------------------------------------------
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] vachan-shetty commented on a change in pull request #13378: [BEAM-11208] Fix for QUOTA_EXCEEDED failures in BQ Storage streams sp…
Posted by GitBox <gi...@apache.org>.
vachan-shetty commented on a change in pull request #13378:
URL: https://github.com/apache/beam/pull/13378#discussion_r526466788
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java
##########
@@ -178,6 +179,7 @@ private BigQueryStorageStreamReader(
this.parseFn = source.parseFn;
this.storageClient = source.bqServices.getStorageClient(options);
this.tableSchema = fromJsonString(source.jsonTableSchema, TableSchema.class);
+ this.splitPossible = true;
Review comment:
Done.
##########
File path: sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryStorageStreamSource.java
##########
@@ -288,78 +290,85 @@ public synchronized void close() {
return null;
}
- SplitReadStreamRequest splitRequest =
- SplitReadStreamRequest.newBuilder()
- .setName(source.readStream.getName())
- .setFraction((float) fraction)
- .build();
-
- SplitReadStreamResponse splitResponse = storageClient.splitReadStream(splitRequest);
- if (!splitResponse.hasPrimaryStream() || !splitResponse.hasRemainderStream()) {
- // No more splits are possible!
- Metrics.counter(
- BigQueryStorageStreamReader.class,
- "split-at-fraction-calls-failed-due-to-impossible-split-point")
- .inc();
- LOG.info(
- "BigQuery Storage API stream {} cannot be split at {}.",
- source.readStream.getName(),
- fraction);
- return null;
- }
+ if (splitPossible) {
Review comment:
Done.
----------------------------------------------------------------
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 #13378: [BEAM-11208] Fix for QUOTA_EXCEEDED failures in BQ Storage streams sp…
Posted by GitBox <gi...@apache.org>.
chamikaramj commented on pull request #13378:
URL: https://github.com/apache/beam/pull/13378#issuecomment-742865754
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