You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "mkuthan (via GitHub)" <gi...@apache.org> on 2023/09/01 08:08:11 UTC

[GitHub] [beam] mkuthan opened a new issue, #28282: [Bug]: BigQuery connector sets insane high maxRetryJobs for unbounded collections

mkuthan opened a new issue, #28282:
URL: https://github.com/apache/beam/issues/28282

   ### What happened?
   
   For unbounded collections and FileLoads writing method default value for maxRetryJobs is 1000. It delays step failure if the error is permanent, for example if TableRow contains timestamp field with invalid value (out of accepted range).
   
   I found the following comment in the code:
   
   ```
   // When running in streaming (unbounded mode) we want to retry failed load jobs
   // indefinitely. Failing the bundle is expensive, so we set a fairly high limit on retries.
   if (IsBounded.UNBOUNDED.equals(input.isBounded())) {
     batchLoads.setMaxRetryJobs(getMaxRetryJobs());
   }
   
   public static <T> Write<T> write() {
       return new AutoValue_BigQueryIO_Write.Builder<T>()
         ...
         .setMaxRetryJobs(1000)
         ...
         .build()
   ```
   
   I would expect:
   * no retries for persistent errors
   * a few retries for transient errors but much less than 1000
   
   In addition the [Documentation](https://beam.apache.org/releases/javadoc/current/org/apache/beam/sdk/io/gcp/bigquery/BigQueryIO.Write.html#withMaxRetryJobs-int-) is far for complete:
   
   * No information that settings is only applicable for FileLoads
   * No information about behaviour if maxRetryJobs is not specified
   
   ### Issue Priority
   
   Priority: 2 (default / most bugs should be filed as P2)
   
   ### Issue Components
   
   - [ ] Component: Python SDK
   - [X] Component: Java SDK
   - [ ] Component: Go SDK
   - [ ] Component: Typescript SDK
   - [ ] Component: IO connector
   - [ ] Component: Beam examples
   - [ ] Component: Beam playground
   - [ ] Component: Beam katas
   - [ ] Component: Website
   - [ ] Component: Spark Runner
   - [ ] Component: Flink Runner
   - [ ] Component: Samza Runner
   - [ ] Component: Twister2 Runner
   - [ ] Component: Hazelcast Jet Runner
   - [ ] Component: Google Cloud Dataflow Runner


-- 
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.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [beam] ahmedabu98 commented on issue #28282: [Bug]: BigQuery connector sets insane high maxRetryJobs for unbounded collections

Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on issue #28282:
URL: https://github.com/apache/beam/issues/28282#issuecomment-1706667770

   This may also get into the territory of needing to add a DLQ for file loads method.. some of these errors may be completely unretriable (e.g. schema mismatch), in which case even a bundle failure won't help


-- 
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] ahmedabu98 commented on issue #28282: [Bug]: BigQuery connector sets insane high maxRetryJobs for unbounded collections

Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on issue #28282:
URL: https://github.com/apache/beam/issues/28282#issuecomment-1706609562

   The decision to retry a job is made in this [block of code](https://github.com/ahmedabu98/beam/blob/262a2dac6c32f1d3f710e5242f728cda525f4a68/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/BigQueryHelpers.java#L305-L346). Perhaps we can parse `jobStatus.getErrorResult()` and fail the bundle for non-transient errors.
   
   @reuvenlax may have better ideas


-- 
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] ahmedabu98 commented on issue #28282: [Bug]: BigQuery connector sets insane high maxRetryJobs for unbounded collections

Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on issue #28282:
URL: https://github.com/apache/beam/issues/28282#issuecomment-1706611092

   >No information that settings is only applicable for FileLoads
   
   FYI in BigQueryIO documentation, "batch loads" is generally synonymous with "file loads"


-- 
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] ahmedabu98 commented on issue #28282: [Bug]: BigQuery connector sets insane high maxRetryJobs for unbounded collections

Posted by "ahmedabu98 (via GitHub)" <gi...@apache.org>.
ahmedabu98 commented on issue #28282:
URL: https://github.com/apache/beam/issues/28282#issuecomment-1706586052

   @liferoad Note the discussion in #27495 is for `STORAGE_WRITE_API` whereas this issue is for `FILE_LOADS` (two different code paths) 


-- 
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] liferoad commented on issue #28282: [Bug]: BigQuery connector sets insane high maxRetryJobs for unbounded collections

Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on issue #28282:
URL: https://github.com/apache/beam/issues/28282#issuecomment-1703959913

   Please check more discussions here: https://github.com/apache/beam/pull/27495


-- 
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] liferoad commented on issue #28282: [Bug]: BigQuery connector sets insane high maxRetryJobs for unbounded collections

Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on issue #28282:
URL: https://github.com/apache/beam/issues/28282#issuecomment-1703959942

   @ahmedabu98 FYI.


-- 
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


Re: [I] [Bug]: BigQuery connector sets insane high maxRetryJobs for unbounded collections [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #28282:
URL: https://github.com/apache/beam/issues/28282#issuecomment-5192106542

   This issue has been marked as stale due to 150 days of inactivity. It will be closed in 30 days if no further activity occurs. If you think that’s incorrect or this issue still needs to be addressed, please simply write any comment. If closed, you can reopen the issue at any time. 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.

To unsubscribe, e-mail: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [Bug]: BigQuery connector sets insane high maxRetryJobs for unbounded collections [beam]

Posted by "mxtymoshyk (via GitHub)" <gi...@apache.org>.
mxtymoshyk commented on issue #28282:
URL: https://github.com/apache/beam/issues/28282#issuecomment-5483211071

   Still relevant, and the code quoted in the description has since moved, so here is where it stands.
   
   ## The documentation half is addressed
   
   Both doc gaps @mkuthan listed are fixed in #39938 (open, pending review), which rewrites the `withMaxRetryJobs` Javadoc:
   
   > Sets the maximum number of times a failed BigQuery load or copy job is retried before the write fails.
   >
   > Only applies when the write method is `FILE_LOADS`. The streaming insert and Storage Write API methods retry at the row level and ignore this setting.
   >
   > If this is not called, a bounded (batch) pipeline retries 3 times and an unbounded (streaming) pipeline retries 1000 times. Streaming defaults higher because failing a bundle in streaming is far more expensive than retrying the load job.
   
   That PR also removes the `.setMaxRetryJobs(1000)` default from `BigQueryIO.write()` that the description quotes. `getMaxRetryJobs()` is now a nullable `Integer` that is unset unless the user calls the setter, which is what let #28281 be fixed without raising the bounded default from 3 to 1000. The two defaults are now named constants, `BatchLoads.DEFAULT_MAX_RETRY_JOBS` (3) and `BatchLoads.DEFAULT_MAX_RETRY_JOBS_UNBOUNDED` (1000).
   
   **The 1000 itself is deliberately untouched by that PR**, because it is this issue.
   
   ## What 1000 actually costs
   
   The backoff between retries is configured in `PendingJobManager` (`BigQueryHelpers.java:120-128`): 1s initial, exponent 1.5, capped at 60s, `withMaxRetries(Integer.MAX_VALUE)` and no `withMaxCumulativeBackoff` override, so `FluentBackoff`'s 1000-day cumulative default never binds. `waitForDone` (`:161-168`) sleeps once per polling round before re-running each still-pending job, and `runJob` gives up when `currentAttempt < maxRetries + 1` fails (`:212-221`, `:287-289`).
   
   Nominal sleep sequence: 1s, 1.5s, 2.25s, ... reaching the 60s cap at the 12th sleep. The first 11 sum to about 171s; the remaining ~988 all sit at the cap for 59,280s.
   
   **Total: roughly 16.5 hours** before a permanently failing load job surfaces as a bundle failure. `FluentBackoff`'s randomization factor of 0.5 puts the real range at roughly 8 to 25 hours. Each of those attempts is a load job that reached a terminal `FAILED` state, not a slow one still running, so this is genuinely 1000 doomed retries of the invalid-timestamp case in the description.
   
   ## The design question is still open
   
   @ahmedabu98 suggested parsing `jobStatus.getErrorResult()` and failing the bundle for non-transient errors, and asked @reuvenlax for better ideas. That was three years ago and there was no reply, so there has never been a decision to implement against.
   
   The options, as far as I can see:
   
   1. **Classify the error.** Fail fast on non-transient BigQuery errors, keep retrying transient ones. Closest to what @ahmedabu98 proposed, and the only one that actually answers "no retries for persistent errors". Needs an agreed list of which reasons are permanent, and there is currently no test coverage for retry behaviour to build on.
   2. **Bound total elapsed time.** Keep the retry count, but stop retrying after some wall-clock budget regardless. Additive and it does not shorten the retry count anyone currently depends on, but it does not distinguish permanent from transient either.
   3. **Lower the constant.** One line, but it silently shortens the retry window for every existing streaming pipeline, including ones riding out BigQuery quota errors. The code comment says the high limit is intentional, so this seems like the least likely to be acceptable.
   
   Worth noting there is currently no test anywhere asserting the unbounded default or any retry count that would move, so whichever direction is taken would want a regression test alongside it.
   
   I have the retry path paged in from working on #28281 and would be glad to implement whichever of these is wanted, but this changes streaming failure behaviour, so it seems like a call for the BigQueryIO maintainers rather than something to guess at. Is there a preferred direction, or should this go to dev@ first? Not claiming the issue in the meantime.
   


-- 
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