Repository navigation
BigQuery: Align metastore default read timeout with the SDK - #18420
fivetran-rahulprakash wants to merge 4 commits into
Conversation
BigQueryMetastoreClientImpl accepted BigQueryOptions but left outgoing requests at the HTTP client's 20-second defaults, ignoring any connect or read timeouts configured in the supplied options. Add a package-private transport-injection constructor that reads HttpTransportOptions from the supplied BigQueryOptions and applies non-negative connect/read timeout values to each outgoing HttpRequest. Zero is forwarded as "no timeout" per the SDK contract; negative values leave the HttpRequest defaults intact. The public constructor delegates to this overload via GoogleNetHttpTransport.newTrustedTransport(), preserving the existing trusted-transport production behavior. Plain BigQueryOptions supplies a 60-second read timeout by default, so honoring it changes BQMS reads from 20 to 60 seconds on that path. Add TestBigQueryMetastoreClientImpl with deterministic regression coverage using an in-memory recording HttpTransport: configured timeouts, SDK defaults vs. explicit-unset options, independent connect/read overrides, zero-forwarding, authentication preservation, 409-to-AlreadyExistsException conversion, and no cross-client leakage.
assertThatThrownBy must include a .hasMessage* check per Iceberg's RegexpMultilineCheck. Add hasMessageContaining to the conflict-response error-conversion test.
Move the org.apache.iceberg.relocated VisibleForTesting import into the org.apache.iceberg import group rather than between com.google.cloud and java imports; spotted by spotlessJavaCheck on CI.
uros-b
left a comment
uros-b
left a comment
There was a problem hiding this comment.
The request-initializer copy is correct for anyone who constructs BigQueryMetastoreClientImpl with BigQueryOptions (>= 0 applied, 0 = no timeout, auth and 409 conversion kept). Through the shipped catalog it is not configurable: BigQueryMetastoreCatalog.initialize uses metastoreOptions(), which never calls setTransportOptions, and the client-injecting initialize(...) is package-private. Catalog users only see the read timeout move from HttpRequest's 20 s to the SDK's 60 s (BigQueryOptions.getDefaultHttpTransportOptions()); connect stays 20 s; there is no way back. A hung list(..., listAllTables=false) load now waits 3× as long; a timed-out commit still goes through checkCommitStatus.
I'd retitle this as aligning with the SDK default read timeout and call out 20 s → 60 s in the body. Catalog properties (gcp.bigquery.http.connect-timeout-ms / read-timeout-ms) can be a follow-up.
Nits if you're already in the test file: take 60 s from getDefaultHttpTransportOptions().getReadTimeout() and 20 s from a plain HttpRequest instead of hardcoding; drop noCrossClientTimeoutLeakage (nothing is static); CapturingTransport keeps the last request, not the first; the "the fix must" comments describe the PR.
Keep timeout coverage aligned with SDK defaults and remove redundant cross-client coverage. Generated-by: GitHub Copilot Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
fivetran-rahulprakash
changed the title
BigQueryMetastoreClientImplnow honors the HTTP timeout values inBigQueryOptions. For standard catalog initialization this aligns the HTTP read timeout with the SDK default, changing it from 20 seconds to 60 seconds; the connect timeout remains 20 seconds. Direct callers can supply custom connect/read timeouts. Negative values retain HTTP-client defaults, and zero means no timeout. Credential initialization, trusted transport, retry policy, and Iceberg error conversion remain unchanged. The standard catalog exposes no timeout properties, so configuring these values through catalog properties is deferred to a follow-up. This does not change uncertain-commit recovery.Test plan: verify configured timeouts on table and dataset requests, SDK defaults versus explicitly unset transport values, independent connect/read overrides, zero forwarding, authentication preservation, and HTTP 409 conversion. Expected defaults are derived from the SDK and unconfigured HTTP requests. The focused tests and complete BigQuery module suite pass, as do Checkstyle and Spotless.
AI Disclosure