3 Commits

Include renames

Author	SHA1	Message	Date
zhangwenjian	a442eadb96	feat✨: keep serving for a configurable window before the listener closes ... /ready has failed from the moment shutdown begins since the readiness probe was added, and the order it does that in is right: reversed, the state would be reported after the connections were already cut. But order alone does not produce a window. Nothing waited between the flip and Shutdown, so the two were microseconds apart, and a poller on a multi-second interval never saw the 503 - it saw a refused connection, which is the thing the probe was supposed to avoid. Polling a container through a SIGTERM on the demo host recorded exactly that: 200, then connection refused, and no 503 in between. extend.shutdown.drain is that wait. The process keeps serving normally for it - answering requests, not refusing them, because refusing them would move the outage earlier rather than avoid it - and only then closes the listener. It is zero by default, so nothing changes for a deployment that does not ask for it. That is not timidity: the budgets are spent one after another, and a non-zero default would push every existing shutdown closer to the orchestrator's grace period, where being cut off part-way through the cleanup callbacks is worse than never draining at all. Keep-alive is switched off with the flip. The server keeps connections alive until Shutdown sets shuttingDown() itself, so without this the pooled connections a balancer holds would sit untouched for the whole window and be cut at the end of it anyway - the cost of the window without its benefit. This is the switch Shutdown flips, moved earlier by the window's length. The signal disposition is restored after the window rather than on the first signal. Before there was a window, the interval where a second signal killed the process outright was only reachable while a cleanup callback hung; putting a multi-second wait inside it would have made every ordinary shutdown interruptible for the length of the drain. A second signal during the window is taken by the channel and ends the window early instead - somebody sending another kill wants this over with sooner - and the escape hatch comes back the moment the window does. What the window is worth depends on who removes this instance. A balancer that polls /ready acts on the 503 and needs the window to cover its check interval times its failure threshold; a Kubernetes Service withdraws the endpoint when the Pod is deleted, concurrently with SIGTERM and regardless of what the probe returns, and there the window covers the delay in that removal reaching every node. The three comments that used to say a balancer "has a chance to" take the instance out said it without either qualification, which is how a claim comes to be repeated after a live test has refuted it. The subprocess test polls the real probes on a connection it opens after the signal - a reused one can be served after the listener is closed, which would let this pass against a shutdown that had already broken it - and asserts on the draining answer in the body, not on the status code. With no database the status is 503 from start-up, so a status-code assertion would hold even with BeginDraining deleted. Two window lengths, because one proves only that something takes that long.	2026-09-06 21:49:17 +08:00
zhangwenjian	f3b67e9abc	fix🐛: keep the rate limiter away from the health probes ... The limiter is installed on the engine and the probes are routes like any other, so above the threshold they are answered with 429 too. Point a liveness probe at one and the failure mode writes itself: traffic crosses the threshold, the probe collects three 429s, the kubelet restarts the container, the capacity that was already short gets shorter, and the instances that are left are pushed further past the threshold. The limiter working exactly as designed is what kills the pod. It is the argument common/health already makes about restarting a process whose database is unreachable, applied to load: turning one outage into a crash loop is not an improvement on the outage. Nothing points a liveness probe at these routes yet. The manifest that will is two commits away, and this has to land first, because that manifest without this change would be actively harmful. The exemption wraps the middleware rather than teaching the limiter about these paths. common/ may not import app/ - the contract check enforces it - so the limiter cannot name routes that are registered over there. Wrapping it in the command package, which imports both, is what keeps the boundary. Naming those routes needs them exported, so the group prefix and the two paths become constants and the router function becomes RegisterMonitorRouter. That also gives a test something real to mount: a probe asserted against a re-implementation of itself is a test of the copy. The check that the middleware never runs is separate from the check that the answer is not 429, because a probe can produce a 429 on its own. What has to be true is that the request never reached the limiter.	2026-09-06 21:49:16 +08:00
zhangwenjian	4e51f56623	feat✨: make the shutdown budgets configurable ... How long a shutdown may spend waiting for in-flight requests, and how long the cleanup callbacks get after that, were compile-time constants. The two together have to fit inside whatever grace period the orchestrator allows before it sends SIGKILL, and that number is not the same everywhere - `docker stop` allows ten seconds, Kubernetes thirty by default - so the one deployment shape these constants suited was the one they were written for. They now come from extend.shutdown, beside rateLimit. Not from application: that section is a fixed struct in core, and the decoder discards keys it has no field for without an error, so a budget written there would be accepted and never applied. That is the failure this whole change is about, and putting the configuration where it cannot be read would have reproduced it. Both fields are pointers, following RateLimit.InboundQPS: nil means "not configured" and takes the default, and a number that was written down is spent literally, zero included. Without that separation `server: 0` - do not wait for in-flight requests at all, which is a reasonable thing to ask when the grace period is very short - could not be expressed, and the section would need a paragraph explaining which zeros mean what. A negative is refused rather than clamped. Correcting a value quietly is the same failure in a different costume, and Budget returns the error instead of ending the process so that the rule can be tested without a subprocess. The defaults live in config as seconds and in cmd/api as durations, both from the same constants, and a test asserts the two agree - a deployment that configures nothing is entitled to one answer about what it spends, not two. The last test loads the two settings files this repository ships through the real loader and asserts the section arrives with the documented values. Nothing weaker can tell "the key is read" from "the key is discarded": the struct compiles either way.	2026-09-06 21:49:16 +08:00