Skip to content

feat(bigtable): fall back to native when the accelerator daemon fails health checks - #22

Open
mutianf wants to merge 1 commit into
accel-19-systest-ci-fixesfrom
accel-20-health-fallback
Open

feat(bigtable): fall back to native when the accelerator daemon fails health checks#22
mutianf wants to merge 1 commit into
accel-19-systest-ci-fixesfrom
accel-20-health-fallback

Conversation

@mutianf

@mutianf mutianf commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Stacked on #21 (accel-19). Consumes the daemon-side grpc.health.v1.Health
service (googleapis/google-cloud-go#20446) and routes to the native client
while the daemon is unwell.

The problem. A starved accelerator daemon is worse than no accelerator: it
is up, it answers, and every call routed to it waits on a process that is not
being scheduled. Nothing routed away from that today — handle_accelerator_error
falls back only on a dead subprocess or UNIMPLEMENTED, so DEADLINE_EXCEEDED
from a wedged daemon surfaced to the caller unchanged.

The probe timeout is the signal, not the payload

A Check touches no session, no Channel and no network, so its latency is
almost entirely the daemon's Go scheduling delay. The daemon deliberately never
self-reports NOT_SERVING under load — it cannot pick that threshold from the
inside — so the caller measures instead. Defaults: 5s interval, 0.25s
deadline
.

This is a stall detector, not a latency monitor, and the module docstring says
so. The probe is issued from the caller's own process, so a busy Python event
loop inflates it exactly as a busy daemon does, and a false positive there
abandons the accelerator for a native path running in the same busy process. A
single-sample deadline cannot separate the two, so the default is set to catch
severe stalls confidently rather than mild degradation unreliably. The milder
regime belongs to the daemon-side pacemaker_delays metric, which measures Go
scheduling delay directly and is already exported.

The breaker is now two flags, not one

_tripped stays permanent (UNIMPLEMENTED, dead subprocess). The new
_degraded is reversible and owned by the monitor. bypass() is the OR.
Keeping them separate is what makes recovery safe: clearing _degraded can
never resurrect an accelerator that trip() has given up on. Three consecutive
probes must agree in either direction, so a GC pause does not flap the route.

UNIMPLEMENTED from the probe stops the loop and leaves the breaker alone.
A daemon too old to serve health checks is unmonitorable, not degraded;
degrading it would disable the accelerator against every daemon predating the
feature.

No new dependency

Python ships the generated health stubs in the separate
grpcio-health-checking distribution, which this package does not require —
Go gets google.golang.org/grpc/health free as a subpackage of an
already-required module. That packaging asymmetry is the whole reason
_accelerator/_health.py exists. The wire format is two trivial messages:
request is b"" (proto3 omits the default service), response is one varint
at field 1.

Verified two independent ways rather than self-consistently: unit tests check
the bytes against google.protobuf.proto_builder (protobuf's own encoder)
instead of restating literals, and the codec was additionally probed end to end
against a throwaway Go binary registering the real
google.golang.org/grpc/health on a UDS.

Wiring

Started in _start_accelerator, stopped in Table.close before the channel it
probes over goes away. The start happens after self._accelerator_daemon = server, so a failure there warns and continues rather than raising and
stranding the subprocess. _use_accelerator() needed no change — it already
consults breaker.bypass().

Testing

34 new tests, including a real grpc.server on a real UDS covering all four
serving statuses, the auth token, timeout, UNIMPLEMENTED, nothing-listening,
and the degrade/recover loop end to end. Full tests/unit/data suite green
(2415 passed, 8 skipped); ruff format --check and flake8 clean on every
touched file.

… checks

A starved accelerator daemon is worse than no accelerator: it is up, it
answers, and every call routed to it waits on a process that is not being
scheduled. Until now nothing routed away from that — `handle_accelerator_error`
only falls back on a dead subprocess or UNIMPLEMENTED, and DEADLINE_EXCEEDED
from a wedged daemon surfaced to the caller unchanged.

This adds a background monitor that polls `grpc.health.v1.Health/Check` on the
daemon's UDS (served as of the daemon-side health-check change) and routes to
the native client while the daemon is unwell.

* **The probe timeout is the signal, not the payload.** A Check touches no
  session, no Channel and no network, so its latency is almost entirely the
  daemon's Go scheduling delay. The daemon deliberately never self-reports
  NOT_SERVING under load — it cannot pick that threshold from the inside — so
  the caller measures instead. 5s interval, 0.25s deadline.

* **The breaker is now two flags, not one.** `_tripped` stays permanent
  (UNIMPLEMENTED, dead subprocess); the new `_degraded` is reversible and owned
  by the monitor. `bypass()` is the OR. Keeping them separate is what makes
  recovery safe: clearing `_degraded` can never resurrect an accelerator that
  `trip()` gave up on. Three consecutive probes must agree in either direction,
  so a GC pause does not flap the route.

* **UNIMPLEMENTED from the probe stops the loop and leaves the breaker alone.**
  A daemon too old to serve health checks is unmonitorable, not degraded;
  treating it as degraded would disable the accelerator against every daemon
  predating the feature.

* **No new dependency.** Python ships the generated health stubs in the
  separate `grpcio-health-checking` distribution, which this package does not
  require (Go gets `google.golang.org/grpc/health` free as a subpackage). The
  wire format is two trivial messages, so `_accelerator/_health.py` encodes
  them directly: request is `b""`, response is one varint at field 1. Tests
  check the bytes against `google.protobuf.proto_builder` — protobuf's own
  encoder — rather than restating literals, and the codec was additionally
  verified end to end against a real Go health server over a UDS.

Wired into `_start_accelerator` (start) and `Table.close` (stop, before the
channel it probes over goes away). Monitor startup runs after the daemon has
been adopted, so a failure there warns and continues rather than raising and
stranding the subprocess. `_use_accelerator()` needed no change: it already
consults `breaker.bypass()`.

34 new tests, including a real gRPC health server on a real UDS. Full
`tests/unit/data` suite green (2415 passed, 8 skipped).

Change-Id: If7a3cded314d9b3926388d50c6a7fb907c81bad1
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