Close the timeout against waits derived from the argument, not only constants
The two bands ruled out a constant and ruled out nothing else. Any mapping that is the identity at 200 and 900 passed for free, and the late tolerance was a flat 300 ms, so anything within that of the truth passed at every value forever. Three ordinary ones survived all five scenarios on both targets: clamping the wait to at most a second, adding 150 ms to it, and scaling it by six fifths. The clamp is the one that matters. Its failure is verbatim the bug the last pass claimed to fix — a host asking for five seconds gets one, forever, with nothing reporting it — and "cap the wait so a shutdown gets noticed" is the most ordinary spelling that line will ever be given. So the long probe goes to 5000, above any ceiling a bridge would plausibly clamp to, which kills the clamp and the scale on the early check and the late one. And the late tolerance becomes a fraction of what was asked for — 60 ms plus an eighth — which is what kills the offset: 150 ms is 75% late at the 200 ms probe and there is no flat gap left to hide in. Measured overshoot on either target is about 10 ms, against 85 ms allowed at the short probe and 685 at the long. The table gets the clamp as an entry of its own, because the whole lesson here is that a table of constants says nothing about a wait computed from the argument, and a table of constants would go green again the moment these numbers drifted back. Its honest limit is stated where the tolerance is defined: a scaling smaller than an eighth survives, and nothing here claims otherwise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
c177533b4f
commit
c682619fbd
2 changed files with 53 additions and 11 deletions
|
|
@ -152,19 +152,40 @@ std::string CloseRaceOnce(int pollers) {
|
|||
// value cannot tell a bridge that honours the host's timeout from one that waits
|
||||
// on that same number of its own: a constant has to satisfy both bands, and no
|
||||
// constant is in both.
|
||||
//
|
||||
// A constant was never the hard case, though, and the numbers are chosen for the
|
||||
// waits that are computed from the argument instead. Any mapping that is the
|
||||
// identity at both of these passes for free, so the two are picked to leave the
|
||||
// ordinary ones nowhere to be identity:
|
||||
//
|
||||
// - 5000 is above any ceiling a bridge would plausibly clamp a host's timeout
|
||||
// to. A wait of `min(timeout, 1000)` — cap it so a shutdown gets noticed — is
|
||||
// the most ordinary spelling this line will ever be given, and it returns
|
||||
// from this probe four seconds early.
|
||||
// - 200 is low enough that an added constant shows as a proportion. A wait of
|
||||
// `timeout + 150` is 75% late here, and the late tolerance below is a
|
||||
// fraction of what was asked for rather than a flat number it could hide in.
|
||||
constexpr int32_t kShortPollMs = 200;
|
||||
constexpr int32_t kLongPollMs = 900;
|
||||
constexpr int32_t kLongPollMs = 5000;
|
||||
|
||||
// How much sooner than the timeout a poll may return. A wait may be late; it may
|
||||
// not be early, because returning early is exactly what waiting on a shorter
|
||||
// duration looks like. This is scheduler granularity, not slack.
|
||||
constexpr int kEarlyToleranceMs = 20;
|
||||
|
||||
// How much later than the timeout a poll may return. Wide enough for a loaded
|
||||
// machine and a container, and narrower than the gap between the two timeouts,
|
||||
// which is what closes the constant off: 200 admits [180, 500] and 900 admits
|
||||
// [880, 1200], and nothing is in both.
|
||||
constexpr int kLateToleranceMs = 300;
|
||||
// How much later than the timeout a poll may return: an allowance for scheduler
|
||||
// granularity plus an eighth of what was asked for.
|
||||
//
|
||||
// Proportional, and not the flat 300 ms this used to be, because a flat
|
||||
// tolerance is a gap a mapping hides in. `timeout + k` for any k under the flat
|
||||
// value passes at every probe, at every value, forever — and the measured
|
||||
// overshoot on both targets is about 10 ms, so a flat 300 was 290 ms of room
|
||||
// for a bridge to add a little of its own and be believed.
|
||||
//
|
||||
// The honest limit: a scaling smaller than an eighth survives this, and this
|
||||
// does not claim otherwise. An eighth is under the smallest scaling anyone
|
||||
// writes on purpose, and 60 ms is six times the overshoot either target shows.
|
||||
constexpr int LateToleranceMs(int32_t timeout_ms) { return 60 + timeout_ms / 8; }
|
||||
|
||||
struct TimedPoll {
|
||||
int32_t result;
|
||||
|
|
@ -209,7 +230,7 @@ std::string PollTimeoutOnce() {
|
|||
" ms: the wait ended on some shorter duration than the one it was given";
|
||||
break;
|
||||
}
|
||||
if (poll.elapsed_ms > timeout_ms + kLateToleranceMs) {
|
||||
if (poll.elapsed_ms > timeout_ms + LateToleranceMs(timeout_ms)) {
|
||||
failure = asked + "returned after " + std::to_string(poll.elapsed_ms) +
|
||||
" ms: the wait ended on some longer duration than the one it was given";
|
||||
break;
|
||||
|
|
|
|||
|
|
@ -478,10 +478,18 @@ const mutations = [
|
|||
// for five seconds silently got two, and every consumer's poll loop spun at two
|
||||
// and a half times the rate it asked for.
|
||||
//
|
||||
// `polltimeout` asks twice, 200 ms and 900 ms, and measures. That is what makes
|
||||
// a constant unsurvivable rather than merely unlucky: no single value sits in
|
||||
// both bands, so the entries below are two sides of one rule and not two
|
||||
// numbers to be tuned against.
|
||||
// `polltimeout` asks twice and measures. That is what makes a constant
|
||||
// unsurvivable rather than merely unlucky: no single value sits in both bands,
|
||||
// so the first two entries below are two sides of one rule and not two numbers
|
||||
// to be tuned against.
|
||||
//
|
||||
// A constant was never the hard case, though. A wait computed from the
|
||||
// argument is identity wherever the probes are, and passes for free: the two
|
||||
// bands rule out a constant and rule out nothing else. Clamping, adding and
|
||||
// scaling all survived the whole matrix once, on bands of 200 and 900 with a
|
||||
// flat late tolerance of 300 ms. The scenario's numbers are what closed that,
|
||||
// and the third entry is here so nothing reopens it quietly — a table of
|
||||
// constants would go green again the moment those numbers drifted back.
|
||||
{
|
||||
target: "native",
|
||||
honestOn: ["linux-x64", "win32-x64"],
|
||||
|
|
@ -498,6 +506,19 @@ const mutations = [
|
|||
old: "std::chrono::milliseconds(timeout_ms),",
|
||||
new: "std::chrono::milliseconds(2000),",
|
||||
},
|
||||
{
|
||||
target: "native",
|
||||
honestOn: ["linux-x64", "win32-x64"],
|
||||
// The derived wait, which is the shape the two constants above do not cover
|
||||
// and the most ordinary spelling this line will ever be given: cap the wait
|
||||
// so a shutdown gets noticed. Its failure is verbatim the one the scenario
|
||||
// exists for — a host asking for five seconds gets one, forever, with
|
||||
// nothing reporting it — and it is identity at any probe below the cap, so
|
||||
// it is the long probe's height that kills it rather than the two bands.
|
||||
rule: "the wait is on the host's timeout (loosening: the bridge caps it at a second)",
|
||||
old: "std::chrono::milliseconds(timeout_ms),",
|
||||
new: "std::chrono::milliseconds((std::min)(timeout_ms, 1000)),",
|
||||
},
|
||||
// The close's wake. It was defended by committed code and had no entry, so the
|
||||
// table understated what `closerace` covers; these say it.
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue