feat(bigtable): fall back to native when the accelerator daemon fails health checks - #22
Open
mutianf wants to merge 1 commit into
Open
feat(bigtable): fall back to native when the accelerator daemon fails health checks#22mutianf wants to merge 1 commit into
mutianf wants to merge 1 commit into
Conversation
… 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #21 (accel-19). Consumes the daemon-side
grpc.health.v1.Healthservice (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_errorfalls back only on a dead subprocess or
UNIMPLEMENTED, soDEADLINE_EXCEEDEDfrom a wedged daemon surfaced to the caller unchanged.
The probe timeout is the signal, not the payload
A
Checktouches no session, no Channel and no network, so its latency isalmost entirely the daemon's Go scheduling delay. The daemon deliberately never
self-reports
NOT_SERVINGunder load — it cannot pick that threshold from theinside — 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_delaysmetric, which measures Goscheduling delay directly and is already exported.
The breaker is now two flags, not one
_trippedstays permanent (UNIMPLEMENTED, dead subprocess). The new_degradedis reversible and owned by the monitor.bypass()is the OR.Keeping them separate is what makes recovery safe: clearing
_degradedcannever resurrect an accelerator that
trip()has given up on. Three consecutiveprobes must agree in either direction, so a GC pause does not flap the route.
UNIMPLEMENTEDfrom 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-checkingdistribution, which this package does not require —Go gets
google.golang.org/grpc/healthfree as a subpackage of analready-required module. That packaging asymmetry is the whole reason
_accelerator/_health.pyexists. The wire format is two trivial messages:request is
b""(proto3 omits the defaultservice), response is one varintat 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/healthon a UDS.Wiring
Started in
_start_accelerator, stopped inTable.closebefore the channel itprobes over goes away. The start happens after
self._accelerator_daemon = server, so a failure there warns and continues rather than raising andstranding the subprocess.
_use_accelerator()needed no change — it alreadyconsults
breaker.bypass().Testing
34 new tests, including a real
grpc.serveron a real UDS covering all fourserving statuses, the auth token, timeout,
UNIMPLEMENTED, nothing-listening,and the degrade/recover loop end to end. Full
tests/unit/datasuite green(2415 passed, 8 skipped);
ruff format --checkandflake8clean on everytouched file.