Skip to content

Bq connector benchmark#4388

Open
squiidz wants to merge 3 commits intomainfrom
bq-connector-benchmark
Open

Bq connector benchmark#4388
squiidz wants to merge 3 commits intomainfrom
bq-connector-benchmark

Conversation

@squiidz
Copy link
Copy Markdown
Contributor

@squiidz squiidz commented May 4, 2026

  ┌───────┬───────────┬─────────┬───────┬──────────┬──────────────┐
  │ batch │ in_flight │ rows/s  │ MB/s  │ ms/batch │ allocs/batch │
  ├───────┼───────────┼─────────┼───────┼──────────┼──────────────┤
  │    10 │         1 │   6,089 │  0.45 │     1.64 │          309 │
  ├───────┼───────────┼─────────┼───────┼──────────┼──────────────┤
  │    10 │         4 │   6,694 │  0.50 │     1.49 │          307 │
  ├───────┼───────────┼─────────┼───────┼──────────┼──────────────┤
  │    10 │        16 │   6,645 │  0.49 │     1.51 │          307 │
  ├───────┼───────────┼─────────┼───────┼──────────┼──────────────┤
  │   100 │         1 │  37,556 │  2.85 │     2.66 │        2,817 │
  ├───────┼───────────┼─────────┼───────┼──────────┼──────────────┤
  │   100 │         4 │  47,214 │  3.58 │     2.12 │        2,817 │
  ├───────┼───────────┼─────────┼───────┼──────────┼──────────────┤
  │   100 │        16 │  52,742 │  4.00 │     1.90 │        2,816 │
  ├───────┼───────────┼─────────┼───────┼──────────┼──────────────┤
  │  1000 │         1 │ 119,284 │  9.28 │     8.38 │       28,884 │
  ├───────┼───────────┼─────────┼───────┼──────────┼──────────────┤
  │  1000 │         4 │ 160,663 │ 12.50 │     6.22 │       28,882 │
  ├───────┼───────────┼─────────┼───────┼──────────┼──────────────┤
  │  1000 │        16 │ 160,973 │ 12.52 │     6.21 │       28,883 │
  └───────┴───────────┴─────────┴───────┴──────────┴──────────────┘

squiidz added 3 commits May 1, 2026 15:30
Schema resolution + evolution
- Add schemaResolver that fetches table metadata, builds a proto
  descriptor via the adapt pipeline, caches per table, and
  deduplicates concurrent cold-cache loads via singleflight.
- Add schemaEvolver that diffs proto descriptor vs table schema and
  adds missing columns. Uses ETag with CAS-on-412 retry (max 5
  attempts) so concurrent additive evolutions don't clobber each
  other; on retry exhaustion or a benign "another writer added the
  columns" outcome, signal the caller to retry the write.
- Add grpcSchemaMismatch classification via StorageError details.
  In handleWriteError: on schema mismatch, evolve and retry; on
  evolution failure, return a permanent BatchError so messages go
  to DLQ instead of being retried forever.
- Add bigquery_write_api_schema_evolutions_total and
  bigquery_write_api_schema_evolution_failures_total counters.

Table name handling
- Sanitize interpolated table names (dots, hyphens, slashes, and
  whitespace become underscores; non-ASCII-alphanumerics are
  stripped; leading digits prefixed with _; cap at 1024 chars).
- Reject empty-after-sanitization names with a permanent BatchError.

Error classification
- Add isPermanentBQError that recognises both gRPC permanent codes
  and *googleapi.Error 4xx (excluding 408/429), so REST 4xx errors
  from the resolver path no longer retry forever.

Lifecycle and concurrency
- Drop bqClient/datasetID fields from resolver and evolver; pass
  client + dataset as method arguments so they can't outlive the
  client. Close drains the resolver cache.
- Track previously fire-and-forget stream closes (evictStream and
  the idle sweeper) on a closeWg so Close waits for them before
  tearing down the underlying clients.
- Snapshot client and resolvedProjectID together under one connMu
  RLock in WriteBatch; pass projectID through to tableCacheKey.
- Run client closes unconditionally in Close — the bigquery and
  managedwriter clients are non-blocking gRPC teardowns; gating
  them on ctx.Err() leaks connections.

Config validation and tuning
- Reject delegates without target_principal at parse time.
- Lower max_in_flight default from 64 to 4 to match snowflake_streaming
  and iceberg sibling outputs.
- Detach the impersonate token source from Connect's ctx with
  context.WithoutCancel so a cancelled connect doesn't break later
  token refreshes.
- Warn explicitly that endpoint overrides disable authentication.

Cleanup
- Drop dead fieldNameMapping plumbing (resolvedSchema field, jsonToProtoBytes
  parameter, streamWithDescriptor field) — never wired in production.
- Drop unused *service.Message argument from Resolve.
- Reject non-positive proto Kind in protoKindToBQFieldType instead of
  silently coercing to STRING; cap RECORD nesting at 15 (BigQuery's
  own limit) to fail fast on self-referential descriptors.
- Set Repeated on field schemas for repeated proto fields.

Tests
- Unit tests for resolver (cache, evict, no-client fallback),
  evolver helpers (kind mapping, schema diff, repeated fields),
  buildAuthOpts (endpoint override, credentials_json, no-auth),
  error classification (gRPC permanent, REST 4xx, wrapped),
  table-name sanitization, sweep goroutine, config validation
  (durations, delegates).
- Integration tests for schema evolution and table-name
  sanitization against the goccy bigquery emulator.
createStream caches the ManagedStream for reuse across every
batch routing to the same table. Bind it to the per-batch ctx
and the first batch's cancellation (per-message deadline, source
shutdown, ack timeout) will sever the cached stream, blocking
every later AppendRows against it until the idle sweeper evicts
the entry.

Wrap with context.WithoutCancel so cancellation propagates only
to the in-flight NewManagedStream dial, not to the long-lived
stream itself.
BenchmarkWriteBatch drives the gcp_bigquery_write_api output end-to-end
against the goccy bigquery emulator, with sub-benchmarks for batch sizes
10/100/1000. Reports rows/s and ms/batch alongside the standard ns/op
and allocations.

The emulator removes real BigQuery's quota and network round-trip cost,
so absolute numbers are not capacity-planning material — but the
benchmark catches local-hot-path regressions (descriptor handling,
batching, gRPC framing, schema-resolver caching) between commits.

Widens startEmulator from *testing.T to testing.TB so it works for
both tests and benchmarks.

Run with:

    go test -run=^BenchmarkWriteBatch$ -bench=^BenchmarkWriteBatch$ \
      -benchtime=10x ./internal/impl/gcp/enterprise/bigquery/
@claude
Copy link
Copy Markdown

claude Bot commented May 4, 2026

Commits

  1. Commit bigquery: add schema resolver, evolution, and table name sanitization bundles a substantial amount of independently-shippable work in one commit (schema resolver, schema evolver, error classification, table-name sanitization, lifecycle/concurrency fixes for closeWg and connMu snapshotting, config validation for delegates, default max_in_flight change from 64→4, dead-code cleanup, plus tests). Commit policy asks for "one small, self-contained, logical change" per commit — splitting the bug fixes (lifecycle, ctx detach, default tuning) from the new features (resolver/evolver, sanitization) and the dead-code cleanup would make this much easier to review and bisect.

Review

The new code is well-structured, well-tested at both unit and integration level, follows project patterns (RCL headers, field-name constants, service.MockResources()+license.InjectTestService, table-driven tests, testcontainers-go with CleanupContainer, Given/When/Then, assert.Eventually without require inside), and the architectural reasoning in commit 2 (context.WithoutCancel for the cached stream so per-batch cancellation can't sever it) and the auth path (context.WithoutCancel for the impersonate token source) is sound. Schema-evolution flow correctly evicts both stream and resolver caches on success, returns permanentBatchError for non-evolvable mismatches and evolution failures, and uses ETag CAS-on-412 with a bounded retry. sanitizeTableName is a single-pass implementation with explicit coverage for the pathological all-underscore input.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant