Skip to content

BigTable: don't collapse the gRPC channel pool to a single channel - #12917

Merged
adutra merged 2 commits into
projectnessie:mainfrom
adutra:bigtable-fixes
Sep 6, 2026
Merged

adutra merged 2 commits into
projectnessie:mainfrom
adutra:bigtable-fixes

Conversation

@adutra

@adutra adutra commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Problem

BigTableClientsFactory.createDataClient() built its channel-pool settings by starting from
ChannelPoolSettings.builder() — gax's generic defaults — and overriding only the properties that
had been explicitly configured. Those generic defaults are:

initial=1 min=1 max=200 minRpcs=0 maxRpcs=MAX_VALUE preemptiveRefresh=false

ChannelPoolSettings.isStaticSize() returns true when
minRpcsPerChannel == 0 && maxRpcsPerChannel == Integer.MAX_VALUE, and for a statically sized pool
ChannelPool never schedules its resize task at all:

if (!settings.isStaticSize()) {
  resizeFuture = ... scheduleAtFixedRate(this::resizeSafely, RESIZE_INTERVAL, ...);
}

So with nothing configured, every Nessie instance ran one gRPC channel to BigTable, permanently,
and maxChannelCount = 200 was dead configuration. Since gRPC starts queuing requests locally
beyond 100 concurrent RPCs per channel, this silently capped BigTable throughput per instance. Under
a burst the queued RPCs consume the request's total timeout and the call fails with
DEADLINE_EXCEEDED, reported as a small residual deadline on the final gax attempt (a clamped
remainder, not a configured timeout — see ExponentialRetryAlgorithm.createNextAttempt).

This also meant each instance held exactly one connection, hence one Google frontend, which makes
"all failing calls resolve to the same frontend IP" an expected consequence rather than a signal.

Change

Derive the pool settings from whatever the BigTable client library has already tuned for the
settings flavour in use, overriding only what is explicitly configured. Verified against
google-cloud-bigtable 2.60.0:

initial min max minRpcs maxRpcs preemptiveRefresh
BigTable library defaults 10 1 200 1 50 true
gax bare defaults (before) 1 1 200 0 MAX_VALUE false

BigtableDataSettings.newBuilder() installs a dynamically sized pool; newBuilderForEmulator()
deliberately installs ChannelPoolSettings.staticallySized(1). Inheriting preserves the intent of
both, where deriving from gax's builder discarded both.

Two notes for reviewers:

  • The hardcoded .setPreemptiveRefreshEnabled(true) is dropped in favour of inheriting it. For the
    real client path this is a no-op (the library default is already true); for the emulator it now
    correctly stays false, consistent with the emulator's setRefreshingChannel(false). This is
    separable from the rest of the change if you would rather keep the explicit call.
  • channelPoolSettings(...) is extracted as a package-private static method so the behaviour can be
    unit-tested without constructing a client.

The Helm chart gains a bigtable.channelPool block, so these no longer have to be set through
advancedConfig.

Threat model

This change alters only client-side connection concurrency to BigTable.

The one security-adjacent dimension is resource consumption: instances will now open more
connections to BigTable than before (10 initially rather than 1, growing under load). This is the
BigTable client library's own default and is bounded by max-channel-count, and it reduces rather
than increases the availability risk the previous behavior created — the old single-channel cap was
itself a self-inflicted denial-of-service under load. Operators who need the old shape can pin
initial-channel-count=1 and max-channel-count=1, as noted in the changelog upgrade notes.

@adutra
adutra requested a review from snazy August 21, 2026 10:25
@snazy

snazy commented Aug 21, 2026

Copy link
Copy Markdown
Member

Inheriting Bigtable’s initial channel count changes the validity of partial existing configurations. For example, max-channel-count=1 used to yield initial=1,max=1; it now yields initial=10,max=1, and ChannelPoolSettings.build() rejects that range. Could we preserve a valid initial value when a configured min/max excludes the inherited one, and add coverage for partial bounds overrides?

Minor follow-up: the new Javadocs say setting minChannelCount equal to itself creates a static pool, and say leaving both RPC thresholds unset does so too. Both statements are incorrect after this change; the relevant equality is min versus max, and unset thresholds inherit Bigtable’s dynamic defaults. I would fix that in the same PR.

adutra added 2 commits August 21, 2026 16:55
Channel pool settings were derived from ChannelPoolSettings.builder() --
gax's generic defaults -- rather than the settings the BigTable client
library has already tuned. Those defaults set min-rpcs-per-channel=0 and
max-rpcs-per-channel=Integer.MAX_VALUE, which makes isStaticSize() return
true, so gax never schedules its resize task and the pool stays pinned at
initial-channel-count (1) for the lifetime of the client. Since gRPC
queues requests locally beyond 100 concurrent RPCs per channel, this
silently capped BigTable throughput per Nessie instance and surfaced as
DEADLINE_EXCEEDED once the queuing exhausted a request's total timeout.

Derive the pool settings from the transport provider already present in
the stub settings, overriding only what is explicitly configured. This
preserves both the tuned dynamic pool installed by
BigtableDataSettings.newBuilder() and the deliberate staticallySized(1)
pool installed by newBuilderForEmulator().

preemptive-refresh is now inherited rather than hardcoded to true: a
no-op for the real client, and correctly false for the emulator.

Also expose the pool settings as bigtable.channelPool in the Helm chart,
so they no longer need to go through advancedConfig.
@adutra
adutra merged commit 1451ff9 into projectnessie:main Sep 6, 2026
16 checks passed
@adutra
adutra deleted the bigtable-fixes branch September 6, 2026 13:58
snazy added a commit that referenced this pull request Sep 22, 2026
…ackport #12917) (#13065)

Channel pool settings were derived from ChannelPoolSettings.builder() --
gax's generic defaults -- rather than the settings the BigTable client
library has already tuned. Those defaults set min-rpcs-per-channel=0 and
max-rpcs-per-channel=Integer.MAX_VALUE, which makes isStaticSize() return
true, so gax never schedules its resize task and the pool stays pinned at
initial-channel-count (1) for the lifetime of the client. Since gRPC
queues requests locally beyond 100 concurrent RPCs per channel, this
silently capped BigTable throughput per Nessie instance and surfaced as
DEADLINE_EXCEEDED once the queuing exhausted a request's total timeout.

Derive the pool settings from the transport provider already present in
the stub settings, overriding only what is explicitly configured. This
preserves both the tuned dynamic pool installed by
BigtableDataSettings.newBuilder() and the deliberate staticallySized(1)
pool installed by newBuilderForEmulator().

preemptive-refresh is now inherited rather than hardcoded to true: a
no-op for the real client, and correctly false for the emulator.

Also expose the pool settings as bigtable.channelPool in the Helm chart,
so they no longer need to go through advancedConfig.

Co-authored-by: Alexandre Dutra <adutra@apache.org>
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.

3 participants