mxtymoshyk opened a new pull request, #39938: URL: https://github.com/apache/beam/pull/39938
`BigQueryIO.Write.withMaxRetryJobs` was silently ignored by every bounded (batch) pipeline. `continueExpandTyped` only forwarded the value to `BatchLoads` when the input was `IsBounded.UNBOUNDED`, so a batch write always used `BatchLoads.DEFAULT_MAX_RETRY_JOBS` (3) no matter what the user asked for. The failure surfaces only as `reached max retries: 3` in the exception text after the job has already given up. The setting is now honored in both modes. Pipelines that never call `withMaxRetryJobs` keep exactly the limits they have today: 3 for bounded, 1000 for unbounded. The Javadoc said only "If set, this will set the max number of retry of batch load jobs." It now states that the setting applies to `FILE_LOADS` only and what the two defaults are, which was the second half of the issue. fixes #28281 ### How the default is preserved `Write.getMaxRetryJobs()` becomes `@Nullable Integer` and is no longer given a value in `BigQueryIO.write()`. Null means "user never asked", which is what makes it possible to honor an explicit value in batch without also raising the batch default from 3 to 1000. This mirrors `getMaxFilesPerBundle` / `getMaxFileSize` in the same `Write` class -- nullable, absent from the `write()` defaults, and applied under a null check at expansion time. The two default values now live next to each other as `BatchLoads.DEFAULT_MAX_RETRY_JOBS` and `BatchLoads.DEFAULT_MAX_RETRY_JOBS_UNBOUNDED`, so the reason streaming retries far more than batch is visible in one place. ### Notes for reviewers * Behaviour for existing users is unchanged unless they call `withMaxRetryJobs`, and if they do call it on a batch pipeline they currently get nothing. * `maxRetryJobs` only ever reaches the `FILE_LOADS` branch of `continueExpandTyped`, so the "only applies to `Method#FILE_LOADS`" sentence in the Javadoc is accurate rather than a guess. * `BigQueryIOTranslation` needed no change: `max_retry_jobs` was already declared with `addNullableInt32Field`, `toConfigRow` can put a null, and `fromConfigRow` already skips a null. `BigQueryIOTranslationTest` passes unchanged. * Public API is unchanged -- `withMaxRetryJobs(int)` keeps its signature. Only the package-private AutoValue accessor and setter changed type. * `testWriteFailedJobs` now asserts `reached max retries: 3` rather than the bare substring. That pins the bounded default so a future change cannot quietly move it. * The new test `testWriteFailedJobsRespectsMaxRetryJobsWhenBounded` reuses the existing `testWriteFailedJobs` harness (`CREATE_NEVER` against a table that does not exist, so every load job attempt fails) and sets `withMaxRetryJobs(1)`. `PendingJob` puts the applied limit into its own failure message, so the assertion reads the value that actually reached `BatchLoads`. A low value also keeps the test fast, since each retry costs a backoff sleep. * Verified locally on `:sdks:java:io:google-cloud-platform`. `BigQueryIOWriteTest.testWriteFailedJobs*`, `testMaxRetryJobs*` and the whole of `BigQueryIOTranslationTest` pass (21 run, 0 failures). Restoring only the old `if (IsBounded.UNBOUNDED...)` guard makes `testWriteFailedJobsRespectsMaxRetryJobsWhenBounded[0]` fail while `testWriteFailedJobs` still passes, so the new test covers the fix and nothing else. * I did not change the unbounded default of 1000, which is the subject of the separate #28282. Asserting it end to end would mean sitting through 1000 backoff-spaced retries, so it is covered only by the null-default path being unchanged. * No prior PR referenced this 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
