fix(dq): port ducklake poison recovery to the query fleet + sustained-NotReady self-exit#22
Merged
Conversation
…-NotReady self-exit (#21) The din-maintenance/inlined-data collision (ducklake v1.5.4 FATAL in DuckLakeInlinedDataReader::TryInitializeScan) poisons query pods: 1736873's two-strike recovery covered only the materializer's decode loop, so a poisoned query pod parked Running-but-NotReady indefinitely — liveness probes the mon port and always passes, and nothing on the GraphQL/fetch/ readiness paths recycled or escalated. Two independent fixes, per the issue: 1. Driver-level two-strike recovery on the query pool (duck/poison.go). The query path has no retry loop to hook, so a connector wrapper observes every QueryContext/ExecContext outcome — GraphQL reads, fetch RPCs, and the /ready probe funnel through one recovery. Same calibration as the materializer's: an aborted catalog session burns the idle pool; "consecutive strike" is judged by connection age (a session created AFTER the recycle that is still aborted proves instance-level poison — exit so the pod supervisor restarts in seconds), so stragglers already in flight on pre-recycle sessions re-burn but never escalate, and a healthy post-recycle session disarms so the next collision starts over at strike one. "database has been invalidated" escalates straight to restart, including at connect time. Recycles are counted in dq_duckdb_poison_recycles_total. The materializer service keeps its own loop recovery and explicitly opts out of the wrapper. 2. Sustained NotReady now exits the process (app/ready.go). A readiness failure sustained for 5 minutes with no intervening success — well past the 60s cascade grace — terminates the process instead of parking NotReady forever, backstopping anything the poison classifier doesn't catch. Not addressed here: the upstream ducklake reader bug (issue fix 3) and shrinking din's flush collision surface (fix 4). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013AeGu5b99yoXAhVcBBxGUw
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013AeGu5b99yoXAhVcBBxGUw
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.
Summary
Fixes #21: the din-maintenance/inlined-data collision (
INTERNAL Error: Attempted to access index 0 within vector of size 0in ducklake v1.5.4'sDuckLakeInlinedDataReader::TryInitializeScan) poisons query pods, and 1736873's two-strike recovery covered only the materializer's decode loop. A poisoned query pod parked Running-but-NotReady indefinitely — liveness probes the mon port and always passes, so nothing ever restarted it (observed ondsn-gcp: both pods 0/1 for ~3 days, x102 readiness failures, first mirrored read traffic 503ing).This implements the issue's two independent in-repo fixes.
1. Driver-level two-strike poison recovery on the query pool (
internal/service/duck/poison.go)The query path has no retry loop to hook (unlike the materializer's
Runloop), so recovery observes errors at thedatabase/sqldriver level: a connector wrapper installed induck.NewServicefunnels everyQueryContext/ExecContextoutcome — GraphQL reads, fetch gRPC, and the/readyprobe — into one recovery, with the same calibration as 1736873:Current transaction is aborted) burns the idle pool so fresh sessions re-bootstrap (re-ATTACH).database has been invalidatedescalates straight to restart, including when it surfaces at connect time.dq_duckdb_poison_recycles_totalmetric so collision frequency is alertable.PoisonRecovery=false): its decode loop keeps the proven 1736873 recovery, and stacking the driver-level one would race it.The wrapper forwards the exact optional-interface set
*duckdb.Connimplements (pinned by a compile-time assertion) sodatabase/sqlbehavior is unchanged for healthy traffic, including prepared statements.2. Sustained NotReady exits the process (
internal/app/ready.go)A readiness failure sustained for 5 minutes with no intervening success terminates the process. It wraps the raw backend check inside
loadTolerantReadiness's cache, so it judges real backing probes; 5m is well past the 60s cascade-grace window, and it backstops anything the poison classifier doesn't catch.Not addressed here
installext; no clearly matching upstream fix found, so reporting upstream with the issue's stack is still warranted.Testing
go build,go vet, and all runnable package tests pass. Thetests/and duck integration suites fail only on this sandbox's blocked extension downloads (HTTP 403 fromextensions.duckdb.org) — verified identical on the pristine tree.🤖 Generated with Claude Code
https://claude.ai/code/session_013AeGu5b99yoXAhVcBBxGUw
Generated by Claude Code