From d3452f3a996f6256ca30f7e74bc249a807e92a09 Mon Sep 17 00:00:00 2001 From: Marko Topolnik Date: Wed, 24 Jun 2026 09:59:37 +0200 Subject: [PATCH 1/6] Adjust timeouts Server start timeout shouldn't be larger than close-drain timeout, because the latter includes the former and will cause the test to falsely blame close-drain, when server start was the slow one. --- system_test/fixture.py | 20 ++++++++++++++------ system_test/qwp_ws_fuzz.py | 28 ++++++++++++++++++++-------- system_test/test.py | 25 +++++++++++++++++++++---- 3 files changed, 55 insertions(+), 18 deletions(-) diff --git a/system_test/fixture.py b/system_test/fixture.py index 793bbf46..8ed23eb6 100644 --- a/system_test/fixture.py +++ b/system_test/fixture.py @@ -528,7 +528,12 @@ def print_log(self): sys.stderr.write(textwrap.indent(log, ' ')) sys.stderr.write('\n\n') - def start(self): + def start(self, start_timeout_sec=300): + # start_timeout_sec bounds the wait for the HTTP service to answer + # /ping. The generous default flags only a genuinely stuck boot; the + # bounce fuzz thread passes a tighter, drain-budget-aware value so a + # pathologically slow restart fails as an infra error rather than + # starving the producers' close_drain. if self.http_server_port is None: ports = discover_avail_ports(3) self.http_server_port, self.line_tcp_port, self.pg_port = ports @@ -640,8 +645,9 @@ def check_http_up(): sys.stderr.write('Waiting until HTTP service is up.\n') retry( check_http_up, - timeout_sec=300, - msg='Timed out waiting for HTTP service to come up.') + timeout_sec=start_timeout_sec, + msg=f'Timed out waiting for HTTP service to come up ' + f'within {start_timeout_sec}s.') except: sys.stderr.write(f'QuestDB log at `{self._log_path}`:\n') self.print_log() @@ -884,7 +890,8 @@ def _env_config(self): env['QDB_LINE_TCP_AUTH_DB_PATH'] = 'conf/auth.txt' return env - def start(self): + def start(self, start_timeout_sec=300): + # start_timeout_sec: see QuestDbFixture.start. if self.http_server_port is None: ports = discover_avail_ports(3) self.http_server_port, self.line_tcp_port, self.pg_port = ports @@ -924,8 +931,9 @@ def start(self): sys.stderr.write('Waiting until HTTP service is up.\n') retry( self._check_http_up, - timeout_sec=300, - msg='Timed out waiting for HTTP service to come up.') + timeout_sec=start_timeout_sec, + msg=f'Timed out waiting for HTTP service to come up ' + f'within {start_timeout_sec}s.') except: sys.stderr.write('QuestDB container log:\n') self.print_log() diff --git a/system_test/qwp_ws_fuzz.py b/system_test/qwp_ws_fuzz.py index 5949ef84..439312da 100644 --- a/system_test/qwp_ws_fuzz.py +++ b/system_test/qwp_ws_fuzz.py @@ -1419,10 +1419,11 @@ class BounceThread(threading.Thread): * the bounce budget is exhausted (``bounces_performed >= max_bounces``); * the producers signal completion via ``writers_done.set()``; * ``stop_event`` is set; or - * a previous bounce raised, in which case ``failure_counter`` gets - bumped and the thread tries one defensive ``stop()`` + ``start()`` - before exiting so the rest of the test still has a server to talk - to (and no half-started instance is left behind). + * a bounce raised — e.g. the server overran ``stop_timeout_s`` on the + way down or ``restart_timeout_s`` on the way back up — in which case + ``failure_counter`` gets bumped and the thread tries one defensive + ``stop()`` + ``start()`` before exiting so the rest of the test still + has a server to talk to (and no half-started instance is left behind). Each bounce is atomic from the caller's perspective: once the loop enters a bounce cycle it completes ``stop()`` + ``start()`` before @@ -1439,6 +1440,7 @@ def __init__( min_interval_s: float, max_interval_s: float, stop_timeout_s: float, + restart_timeout_s: float, writers_done: threading.Event, stop_event: threading.Event, record_failure, @@ -1452,6 +1454,7 @@ def __init__( self._min_interval_s = min_interval_s self._max_interval_s = max(max_interval_s, min_interval_s) self._stop_timeout_s = stop_timeout_s + self._restart_timeout_s = restart_timeout_s self._writers_done = writers_done self._stop_event = stop_event self._record_failure = record_failure @@ -1486,16 +1489,25 @@ def run(self): # before start() rebinds them. time.sleep(0.02 + self._rnd.next_int(200) / 1000.0) self._log(f'fuzz bounce #{idx}: starting QDB') - self._fixture.start() + # Cap the restart wait below the producers' close_drain + # budget: a restart slower than this leaves them no window + # to replay, so a timeout here is the trustworthy, + # correctly-attributed signal (server too slow to restart) + # rather than a downstream client close_drain timeout. + self._fixture.start(start_timeout_sec=self._restart_timeout_s) self.bounces_performed += 1 self._log(f'fuzz bounce #{idx}: server back up') except Exception as e: # noqa: BLE001 — any lifecycle failure fails the run # A raise here is a real failure, not noise: stop() raises # when the server won't shut down within its timeout, and - # start() raises when it won't come back up. Either way we - # record it so the end-of-run assertion fails. + # start() raises when it won't come back up within + # restart_timeout_s. Both bounds are sized so only a stuck + # or pathologically slow server trips them; a restart that + # slow starves the producers' close_drain, so we attribute + # it here instead of letting it resurface as a misleading + # client timeout. self._record_failure( - f'fuzz bounce: unexpected failure at attempt ' + f'fuzz bounce: server lifecycle failed at attempt ' f'{self.bounces_performed + 1}: ' f'{type(e).__name__}: {e}') # One defensive recovery attempt so the rest of the test diff --git a/system_test/test.py b/system_test/test.py index 95585991..602fdaec 100755 --- a/system_test/test.py +++ b/system_test/test.py @@ -2302,6 +2302,20 @@ class TestQwpWsFuzz(QwpWsTestSupport, unittest.TestCase): POLL_INTERVAL_SEC = 0.05 DRAIN_TIMEOUT_SEC = 120 + # Producers cap close_drain() at CLOSE_FLUSH_TIMEOUT_SEC: the wall-clock + # budget for the client to reconnect after a bounce and replay every + # queued frame into the restarted server. A bounce restart must finish + # well inside that budget, or the producers never get a window to drain + # and time out — surfacing a server-too-slow event as a misleading + # client-side close_drain timeout. So the bounce thread caps each + # restart at BOUNCE_RESTART_TIMEOUT_SEC and fails the run with a + # correctly-attributed infra error when it's exceeded. Keep it well + # above a healthy restart (~6-40s on CI) and below + # CLOSE_FLUSH_TIMEOUT_SEC so the restart cap, not the drain, is what + # trips first. + CLOSE_FLUSH_TIMEOUT_SEC = 120 + BOUNCE_RESTART_TIMEOUT_SEC = 90 + def setUp(self): self._require_fuzz_fixture() seed = qwp_ws_fuzz.derive_master_seed() @@ -2509,6 +2523,7 @@ def record_failure(msg: str): min_interval_s=fuzz.min_bounce_interval_s, max_interval_s=fuzz.max_bounce_interval_s, stop_timeout_s=fuzz.bounce_stop_timeout_s, + restart_timeout_s=self.BOUNCE_RESTART_TIMEOUT_SEC, writers_done=producers_done, stop_event=stop_event, record_failure=record_failure, @@ -2590,10 +2605,12 @@ def _producer_loop(self, sender_id, sf_root, load, fuzz, rnd, # the non-bounce tests, which never reconnect, so the backoff # never engages.) reconnect_max_backoff_millis=250, - # 2 min on close_drain — bounce-test variants need a long - # enough budget for SFA to replay queued frames into a - # freshly-restarted server. - close_flush_timeout_millis=120000) + # close_drain budget: long enough for SFA to replay queued + # frames into a freshly-restarted server. A bounce restart is + # capped below this (BOUNCE_RESTART_TIMEOUT_SEC) so a slow + # restart fails as an infra error instead of starving this + # drain. + close_flush_timeout_millis=self.CLOSE_FLUSH_TIMEOUT_SEC * 1000) try: sender = self._connect_sender(conf) except Exception as e: # noqa: BLE001 From aa70079722f9ccae868075cdbfe9bf6d30a7fa9c Mon Sep 17 00:00:00 2001 From: Marko Topolnik Date: Wed, 24 Jun 2026 10:39:04 +0200 Subject: [PATCH 2/6] Use min-HTTP health check instead of /ping /ping is served from the shared thread pool, and gets starved under severe ingestion load, making the server appear not-started to the fuzz test harness --- system_test/fixture.py | 55 ++++++++++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/system_test/fixture.py b/system_test/fixture.py index 8ed23eb6..b801856a 100644 --- a/system_test/fixture.py +++ b/system_test/fixture.py @@ -504,6 +504,11 @@ def __init__( self.line_tcp_port = None self.qwp_udp_port = None self.pg_port = None + # Dedicated min-HTTP health port. The min server runs on its own + # worker pool, so the readiness probe can't be starved by QWP + # ingest saturating the shared-network pool that serves the main + # HTTP server (where /ping lives). + self.http_min_port = None self.wrap_tls = wrap_tls self._tls_proxy = None @@ -529,14 +534,14 @@ def print_log(self): sys.stderr.write('\n\n') def start(self, start_timeout_sec=300): - # start_timeout_sec bounds the wait for the HTTP service to answer - # /ping. The generous default flags only a genuinely stuck boot; the - # bounce fuzz thread passes a tighter, drain-budget-aware value so a - # pathologically slow restart fails as an infra error rather than - # starving the producers' close_drain. + # start_timeout_sec bounds the wait for the min-HTTP health endpoint + # to answer. The generous default flags only a genuinely stuck boot; + # the bounce fuzz thread passes a tighter, drain-budget-aware value + # so a pathologically slow restart fails as an infra error rather + # than starving the producers' close_drain. if self.http_server_port is None: - ports = discover_avail_ports(3) - self.http_server_port, self.line_tcp_port, self.pg_port = ports + (self.http_server_port, self.line_tcp_port, + self.pg_port, self.http_min_port) = discover_avail_ports(4) if self.qwp_udp and self.qwp_udp_port is None: self.qwp_udp_port = discover_avail_udp_port() auth_config = 'line.tcp.auth.db.path=conf/auth.txt' if self.auth else '' @@ -557,7 +562,9 @@ def start(self, start_timeout_sec=300): http.bind.to=0.0.0.0:{self.http_server_port} line.tcp.net.bind.to=0.0.0.0:{self.line_tcp_port} pg.net.bind.to=0.0.0.0:{self.pg_port} - http.min.enabled=false + http.min.enabled=true + http.min.net.bind.to=0.0.0.0:{self.http_min_port} + http.min.worker.count=1 line.udp.enabled=false qwp.udp.enabled={qwp_udp_enabled} {qwp_udp_bind} @@ -628,13 +635,18 @@ def start(self, start_timeout_sec=300): def check_http_up(): if self._proc.poll() is not None: raise RuntimeError('QuestDB died during startup.') + # Probe the dedicated min-HTTP health endpoint, not the main + # HTTP /ping: the min server runs on its own worker, so a + # heavy QWP ingest load on the shared-network pool can't + # starve the readiness check. HealthCheckProcessor answers + # any path with 200 "Status: Healthy". req = urllib.request.Request( - f'http://127.0.0.1:{self.http_server_port}/ping', + f'http://127.0.0.1:{self.http_min_port}/status', headers=self.http_headers(), method='GET') try: resp = urllib.request.urlopen(req, timeout=1) - if resp.status == 204: + if 200 <= resp.status < 300: return True except socket.timeout: pass @@ -662,8 +674,8 @@ def check_http_up(): # doesn't change across restarts, and a bounce restart must stay # clear of SQL — right after it the reconnecting fuzz producers # can keep the network workers busy past the 5s query timeout, - # failing the bounce even though /ping already vouched for - # liveness. + # failing the bounce even though the min-HTTP health endpoint + # already vouched for liveness. if not self._version_queried: self.version = self.query_version() self._version_queried = True @@ -843,6 +855,7 @@ def __init__( self.line_tcp_port = None self.qwp_udp_port = None self.pg_port = None + self.http_min_port = None # see QuestDbFixture.__init__ self.wrap_tls = wrap_tls self._tls_proxy = None @@ -863,7 +876,9 @@ def _env_config(self): 'QDB_HTTP_BIND_TO': f'0.0.0.0:{self.http_server_port}', 'QDB_LINE_TCP_NET_BIND_TO': f'0.0.0.0:{self.line_tcp_port}', 'QDB_PG_NET_BIND_TO': f'0.0.0.0:{self.pg_port}', - 'QDB_HTTP_MIN_ENABLED': 'false', + 'QDB_HTTP_MIN_ENABLED': 'true', + 'QDB_HTTP_MIN_NET_BIND_TO': f'0.0.0.0:{self.http_min_port}', + 'QDB_HTTP_MIN_WORKER_COUNT': '1', 'QDB_LINE_UDP_ENABLED': 'false', 'QDB_QWP_UDP_ENABLED': 'true' if self.qwp_udp else 'false', 'QDB_LINE_TCP_MAINTENANCE_JOB_INTERVAL': '100', @@ -893,8 +908,8 @@ def _env_config(self): def start(self, start_timeout_sec=300): # start_timeout_sec: see QuestDbFixture.start. if self.http_server_port is None: - ports = discover_avail_ports(3) - self.http_server_port, self.line_tcp_port, self.pg_port = ports + (self.http_server_port, self.line_tcp_port, + self.pg_port, self.http_min_port) = discover_avail_ports(4) if self.qwp_udp and self.qwp_udp_port is None: self.qwp_udp_port = discover_avail_udp_port() @@ -902,7 +917,8 @@ def start(self, start_timeout_sec=300): self._container = cmd[3] for key, value in self._env_config().items(): cmd += ['-e', f'{key}={value}'] - for port in (self.http_server_port, self.line_tcp_port, self.pg_port): + for port in (self.http_server_port, self.line_tcp_port, + self.pg_port, self.http_min_port): cmd += ['-p', f'127.0.0.1:{port}:{port}'] if self.qwp_udp: cmd += ['-p', f'127.0.0.1:{self.qwp_udp_port}:{self.qwp_udp_port}/udp'] @@ -961,13 +977,16 @@ def _is_running(self): def _check_http_up(self): if not self._is_running(): raise RuntimeError('QuestDB container died during startup.') + # Probe the dedicated min-HTTP health endpoint (see + # QuestDbFixture.start): isolated from the shared-network pool that + # serves the main HTTP server and QWP ingest. req = urllib.request.Request( - f'http://{self.host}:{self.http_server_port}/ping', + f'http://{self.host}:{self.http_min_port}/status', headers=self.http_headers(), method='GET') try: resp = urllib.request.urlopen(req, timeout=1) - if resp.status == 204: + if 200 <= resp.status < 300: return True except urllib.error.URLError: pass From 7faa411e07e1a5d9cd0635d069490683274bd323 Mon Sep 17 00:00:00 2001 From: Marko Topolnik Date: Wed, 24 Jun 2026 10:58:11 +0200 Subject: [PATCH 3/6] Share min-HTTP readiness probe across fixtures The managed and docker fixtures each carried a near-identical copy of the min-HTTP readiness probe: same request to /status on the dedicated min-HTTP port, same 200-range status check, and the same rationale comment. Keeping the two in sync by hand risks fixture-type-dependent flakiness if one copy is changed and the other is missed. Hoist the probe into a single QuestDbFixtureBase._check_http_up(). The only fixture-specific part, the liveness pre-check, becomes an _assert_server_alive() hook that each subclass overrides (the managed fixture inspects the process, the docker fixture inspects the container). The base method uses self.host instead of a hardcoded 127.0.0.1, which QuestDbFixture already sets to that value. The two handlers were not identical: the managed copy caught socket.timeout and URLError, while the docker copy also caught OSError to absorb the connection reset that happens when docker publishes the host port before QuestDB binds. Since URLError and socket.timeout are both OSError subclasses, a single `except OSError` is the exact union, so the consolidated probe keeps both behaviors. Co-Authored-By: Claude Opus 4.8 (1M context) --- system_test/fixture.py | 86 ++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 46 deletions(-) diff --git a/system_test/fixture.py b/system_test/fixture.py index b801856a..13f3ba8b 100644 --- a/system_test/fixture.py +++ b/system_test/fixture.py @@ -369,6 +369,40 @@ def http_sql_query(self, sql_query): raise QueryError(data['error']) return data + def _assert_server_alive(self): + """Raise if the server died during startup. + + The readiness probe calls this before each attempt so a crashed + server fails fast instead of looping until the timeout. The base + fixture has no process to inspect; the managed and docker fixtures + override it. + """ + + def _check_http_up(self): + # Probe the dedicated min-HTTP health endpoint, not the main HTTP + # server: the min server runs on its own worker pool, so a heavy QWP + # ingest load on the shared-network pool (which serves the main HTTP + # server) can't starve the readiness check. HealthCheckProcessor + # answers any path with 200 "Status: Healthy". + self._assert_server_alive() + req = urllib.request.Request( + f'http://{self.host}:{self.http_min_port}/status', + headers=self.http_headers(), + method='GET') + try: + resp = urllib.request.urlopen(req, timeout=1) + if 200 <= resp.status < 300: + return True + except OSError: + # The server isn't accepting yet. A not-yet-ready local process + # refuses the connection (urllib.error.URLError); docker + # publishes the host port before QuestDB binds, so an early probe + # can connect and then be reset (RemoteDisconnected / + # ConnectionResetError). URLError and socket.timeout are both + # OSError subclasses, so one handler covers every case. + pass + return False + def query_version(self): try: res = self.http_sql_query('select build') @@ -533,6 +567,10 @@ def print_log(self): sys.stderr.write(textwrap.indent(log, ' ')) sys.stderr.write('\n\n') + def _assert_server_alive(self): + if self._proc.poll() is not None: + raise RuntimeError('QuestDB died during startup.') + def start(self, start_timeout_sec=300): # start_timeout_sec bounds the wait for the min-HTTP health endpoint # to answer. The generous default flags only a genuinely stuck boot; @@ -632,31 +670,9 @@ def start(self, start_timeout_sec=300): stderr=subprocess.STDOUT, creationflags=creationflags) - def check_http_up(): - if self._proc.poll() is not None: - raise RuntimeError('QuestDB died during startup.') - # Probe the dedicated min-HTTP health endpoint, not the main - # HTTP /ping: the min server runs on its own worker, so a - # heavy QWP ingest load on the shared-network pool can't - # starve the readiness check. HealthCheckProcessor answers - # any path with 200 "Status: Healthy". - req = urllib.request.Request( - f'http://127.0.0.1:{self.http_min_port}/status', - headers=self.http_headers(), - method='GET') - try: - resp = urllib.request.urlopen(req, timeout=1) - if 200 <= resp.status < 300: - return True - except socket.timeout: - pass - except urllib.error.URLError: - pass - return False - sys.stderr.write('Waiting until HTTP service is up.\n') retry( - check_http_up, + self._check_http_up, timeout_sec=start_timeout_sec, msg=f'Timed out waiting for HTTP service to come up ' f'within {start_timeout_sec}s.') @@ -974,31 +990,9 @@ def _is_running(self): check=False, capture=True) return res.returncode == 0 and res.stdout.strip() == b'true' - def _check_http_up(self): + def _assert_server_alive(self): if not self._is_running(): raise RuntimeError('QuestDB container died during startup.') - # Probe the dedicated min-HTTP health endpoint (see - # QuestDbFixture.start): isolated from the shared-network pool that - # serves the main HTTP server and QWP ingest. - req = urllib.request.Request( - f'http://{self.host}:{self.http_min_port}/status', - headers=self.http_headers(), - method='GET') - try: - resp = urllib.request.urlopen(req, timeout=1) - if 200 <= resp.status < 300: - return True - except urllib.error.URLError: - pass - except OSError: - # Docker publishes the host port before QuestDB is ready, so an - # early probe can connect and then be reset - # (http.client.RemoteDisconnected, a ConnectionResetError / - # OSError). The local-process fixture only sees connection-refused - # (URLError) because its port opens with the app. socket.timeout - # is an OSError too. - pass - return False def __enter__(self): self.start() From 833196a5c0cf85a6cd77535155a1e7e97abff499 Mon Sep 17 00:00:00 2001 From: Marko Topolnik Date: Wed, 24 Jun 2026 12:53:25 +0200 Subject: [PATCH 4/6] Probe main HTTP on initial start, min on bounce The readiness probe was switched to the min-HTTP /status endpoint to keep a bounce restart's probe from being starved by reconnecting producers on the shared-network worker pool. But start() refines the build version via a query to the main HTTP server right after the probe passes, and the whole suite runs its SQL there. On a cold boot the dedicated min-HTTP pool answers seconds before the main HTTP server on the shared pool begins accepting, so query_version() hit a not-yet-listening port and aborted startup with ConnectionRefusedError on slow CI boxes. Gate readiness on the server each start phase actually uses. The initial start probes main HTTP /ping, restoring the guarantee that the main server is accepting before query_version() and the test SQL run; no ingest exists yet, so /ping cannot be starved. A bounce restart passes probe_min_http=True to keep gating on the starvation-proof min-HTTP pool, and that path issues no SQL after the probe. Also bound the defensive recovery restart to restart_timeout_s and the min-HTTP probe, so a still-reconnecting producer storm cannot starve it and the daemon thread cannot outlive the test's join budget. Co-Authored-By: Claude Opus 4.8 (1M context) --- system_test/fixture.py | 61 ++++++++++++++++++++++++++++---------- system_test/qwp_ws_fuzz.py | 20 +++++++++++-- 2 files changed, 62 insertions(+), 19 deletions(-) diff --git a/system_test/fixture.py b/system_test/fixture.py index 13f3ba8b..fea86c16 100644 --- a/system_test/fixture.py +++ b/system_test/fixture.py @@ -378,20 +378,34 @@ def _assert_server_alive(self): override it. """ - def _check_http_up(self): - # Probe the dedicated min-HTTP health endpoint, not the main HTTP - # server: the min server runs on its own worker pool, so a heavy QWP + def _check_main_http_up(self): + # Probe the main HTTP server's /ping. The initial start gates on + # this: query_version() and every test SQL query run against the main + # HTTP server, so it must be accepting before start() returns. There + # is no QWP ingest at initial-start time, so /ping can't be starved + # by the shared-network pool that serves it. + return self._probe_http( + self.http_server_port, '/ping', lambda status: status == 204) + + def _check_min_http_up(self): + # Probe the dedicated min-HTTP health endpoint. A bounce restart gates + # on this: the min server runs on its own worker pool, so a heavy QWP # ingest load on the shared-network pool (which serves the main HTTP # server) can't starve the readiness check. HealthCheckProcessor - # answers any path with 200 "Status: Healthy". + # answers any path with 200 "Status: Healthy". The bounce path issues + # no SQL after the probe, so main-HTTP readiness isn't required there. + return self._probe_http( + self.http_min_port, '/status', lambda status: 200 <= status < 300) + + def _probe_http(self, port, path, status_ok): self._assert_server_alive() req = urllib.request.Request( - f'http://{self.host}:{self.http_min_port}/status', + f'http://{self.host}:{port}{path}', headers=self.http_headers(), method='GET') try: resp = urllib.request.urlopen(req, timeout=1) - if 200 <= resp.status < 300: + if status_ok(resp.status): return True except OSError: # The server isn't accepting yet. A not-yet-ready local process @@ -571,12 +585,21 @@ def _assert_server_alive(self): if self._proc.poll() is not None: raise RuntimeError('QuestDB died during startup.') - def start(self, start_timeout_sec=300): - # start_timeout_sec bounds the wait for the min-HTTP health endpoint - # to answer. The generous default flags only a genuinely stuck boot; - # the bounce fuzz thread passes a tighter, drain-budget-aware value - # so a pathologically slow restart fails as an infra error rather - # than starving the producers' close_drain. + def start(self, start_timeout_sec=300, probe_min_http=False): + # start_timeout_sec bounds the wait for the readiness probe to pass. + # The generous default flags only a genuinely stuck boot; the bounce + # fuzz thread passes a tighter, drain-budget-aware value so a + # pathologically slow restart fails as an infra error rather than + # starving the producers' close_drain. + # + # probe_min_http selects which server gates readiness. The initial + # start probes the main HTTP server (/ping): query_version() and all + # test SQL hit it, so it must be accepting before start() returns, + # and there is no ingest yet to starve it. A bounce restart sets + # probe_min_http=True to gate on the dedicated min-HTTP pool instead, + # which reconnecting producers on the shared-network pool can't + # starve; that path issues no SQL, so main-HTTP readiness isn't + # needed. if self.http_server_port is None: (self.http_server_port, self.line_tcp_port, self.pg_port, self.http_min_port) = discover_avail_ports(4) @@ -671,8 +694,11 @@ def start(self, start_timeout_sec=300): creationflags=creationflags) sys.stderr.write('Waiting until HTTP service is up.\n') + check_up = ( + self._check_min_http_up if probe_min_http + else self._check_main_http_up) retry( - self._check_http_up, + check_up, timeout_sec=start_timeout_sec, msg=f'Timed out waiting for HTTP service to come up ' f'within {start_timeout_sec}s.') @@ -921,8 +947,8 @@ def _env_config(self): env['QDB_LINE_TCP_AUTH_DB_PATH'] = 'conf/auth.txt' return env - def start(self, start_timeout_sec=300): - # start_timeout_sec: see QuestDbFixture.start. + def start(self, start_timeout_sec=300, probe_min_http=False): + # start_timeout_sec / probe_min_http: see QuestDbFixture.start. if self.http_server_port is None: (self.http_server_port, self.line_tcp_port, self.pg_port, self.http_min_port) = discover_avail_ports(4) @@ -961,8 +987,11 @@ def start(self, start_timeout_sec=300): try: sys.stderr.write('Waiting until HTTP service is up.\n') + check_up = ( + self._check_min_http_up if probe_min_http + else self._check_main_http_up) retry( - self._check_http_up, + check_up, timeout_sec=start_timeout_sec, msg=f'Timed out waiting for HTTP service to come up ' f'within {start_timeout_sec}s.') diff --git a/system_test/qwp_ws_fuzz.py b/system_test/qwp_ws_fuzz.py index 439312da..9dcc6407 100644 --- a/system_test/qwp_ws_fuzz.py +++ b/system_test/qwp_ws_fuzz.py @@ -1494,7 +1494,15 @@ def run(self): # to replay, so a timeout here is the trustworthy, # correctly-attributed signal (server too slow to restart) # rather than a downstream client close_drain timeout. - self._fixture.start(start_timeout_sec=self._restart_timeout_s) + # + # Gate on the min-HTTP pool, not main /ping: the producers + # are reconnecting and saturating the shared-network pool + # that serves main HTTP, so a main-HTTP probe here would be + # starved and time out even on a healthy restart. The + # restart issues no SQL, so main-HTTP readiness isn't needed. + self._fixture.start( + start_timeout_sec=self._restart_timeout_s, + probe_min_http=True) self.bounces_performed += 1 self._log(f'fuzz bounce #{idx}: server back up') except Exception as e: # noqa: BLE001 — any lifecycle failure fails the run @@ -1514,13 +1522,19 @@ def run(self): # has a chance to surface the underlying assertion failure # rather than a query timeout. stop() first: if start() # failed partway it may have left a process behind, and we - # must not launch a second one next to it. + # must not launch a second one next to it. Reuse the bounded + # restart timeout and the min-HTTP probe: producers may still + # be reconnecting, so a main-HTTP probe would be starved, and + # an unbounded wait would let this daemon thread outlive the + # test's join budget. try: self._fixture.stop(wait_timeout_sec=self._stop_timeout_s) except Exception: pass try: - self._fixture.start() + self._fixture.start( + start_timeout_sec=self._restart_timeout_s, + probe_min_http=True) except Exception: pass return From 3800005ee3ea02f1eaa2e86fd7e91d99c6c77f55 Mon Sep 17 00:00:00 2001 From: Marko Topolnik Date: Wed, 24 Jun 2026 13:35:20 +0200 Subject: [PATCH 5/6] Size fuzz bounce timeouts to the full down-window The fuzz bounce test's timing budgets assumed only the restart leg of a bounce could starve the client, but a bounce makes the server unavailable for a whole stop() + start() cycle. With the producer close_drain/reconnect budget and the restart cap both around 120s, a slow graceful shutdown could burn a producer's entire drain window and surface as a misleading client-side close_drain timeout, with no bounce failure recorded. Size the budgets to the real worst case instead: - Derive the producer reconnect and close_drain budget from bounce_stop_timeout_s + BOUNCE_RESTART_TIMEOUT_SEC (plus headroom) and apply it to both knobs, so a healthy-but-slow bounce can never strand a producer mid-drain. Keeping reconnect and drain equal stops a reconnect from giving up mid-window while the drain still has budget. - Size the bounce-thread wind-down join to a full cycle and record a failure if the thread is still alive afterward, so a stuck lifecycle is attributed here instead of racing the verification queries and teardown on shared fixture state. Lowering bounce_stop_timeout_s was rejected: it risks force-killing a server still draining its WAL backlog. Raising the client budgets only slows a genuine failure (now up to ~240s), never a passing run, since the ceilings are reached only by a genuinely stuck server. Co-Authored-By: Claude Opus 4.8 (1M context) --- system_test/qwp_ws_fuzz.py | 28 ++++++++----- system_test/test.py | 84 +++++++++++++++++++++++++------------- 2 files changed, 72 insertions(+), 40 deletions(-) diff --git a/system_test/qwp_ws_fuzz.py b/system_test/qwp_ws_fuzz.py index 9dcc6407..75a91b31 100644 --- a/system_test/qwp_ws_fuzz.py +++ b/system_test/qwp_ws_fuzz.py @@ -1489,11 +1489,14 @@ def run(self): # before start() rebinds them. time.sleep(0.02 + self._rnd.next_int(200) / 1000.0) self._log(f'fuzz bounce #{idx}: starting QDB') - # Cap the restart wait below the producers' close_drain - # budget: a restart slower than this leaves them no window - # to replay, so a timeout here is the trustworthy, - # correctly-attributed signal (server too slow to restart) - # rather than a downstream client close_drain timeout. + # Cap the restart wait at restart_timeout_s: a restart that + # overruns it is a stuck boot, and failing here gives the + # trustworthy, correctly-attributed signal (server too slow + # to restart). The producers' reconnect/close_drain budgets + # are sized above a full stop()+start() down-window (see + # TestQwpWsFuzz._producer_loop), so a restart inside this cap + # never strands them — only one that trips the cap does, and + # that surfaces here, not as a downstream client timeout. # # Gate on the min-HTTP pool, not main /ping: the producers # are reconnecting and saturating the shared-network pool @@ -1510,10 +1513,10 @@ def run(self): # when the server won't shut down within its timeout, and # start() raises when it won't come back up within # restart_timeout_s. Both bounds are sized so only a stuck - # or pathologically slow server trips them; a restart that - # slow starves the producers' close_drain, so we attribute - # it here instead of letting it resurface as a misleading - # client timeout. + # or pathologically slow server trips them, and a server that + # slow is exactly what the producers' down-window budget is + # sized to outlast — so attributing it here, rather than as a + # downstream client timeout, is the correct call. self._record_failure( f'fuzz bounce: server lifecycle failed at attempt ' f'{self.bounces_performed + 1}: ' @@ -1525,8 +1528,11 @@ def run(self): # must not launch a second one next to it. Reuse the bounded # restart timeout and the min-HTTP probe: producers may still # be reconnecting, so a main-HTTP probe would be starved, and - # an unbounded wait would let this daemon thread outlive the - # test's join budget. + # the bound keeps this recovery from running unbounded. The + # failure is already recorded, so even if recovery overruns + # the test's bounce wind-down the run still fails with correct + # attribution; the wind-down's is_alive check flags a thread + # still stuck here. try: self._fixture.stop(wait_timeout_sec=self._stop_timeout_s) except Exception: diff --git a/system_test/test.py b/system_test/test.py index 602fdaec..12a19842 100755 --- a/system_test/test.py +++ b/system_test/test.py @@ -2302,18 +2302,22 @@ class TestQwpWsFuzz(QwpWsTestSupport, unittest.TestCase): POLL_INTERVAL_SEC = 0.05 DRAIN_TIMEOUT_SEC = 120 - # Producers cap close_drain() at CLOSE_FLUSH_TIMEOUT_SEC: the wall-clock - # budget for the client to reconnect after a bounce and replay every - # queued frame into the restarted server. A bounce restart must finish - # well inside that budget, or the producers never get a window to drain - # and time out — surfacing a server-too-slow event as a misleading - # client-side close_drain timeout. So the bounce thread caps each - # restart at BOUNCE_RESTART_TIMEOUT_SEC and fails the run with a - # correctly-attributed infra error when it's exceeded. Keep it well - # above a healthy restart (~6-40s on CI) and below - # CLOSE_FLUSH_TIMEOUT_SEC so the restart cap, not the drain, is what - # trips first. - CLOSE_FLUSH_TIMEOUT_SEC = 120 + # A bounce makes the server unavailable for a whole stop() + start() + # cycle. BOUNCE_RESTART_TIMEOUT_SEC caps only the start (restart) leg; + # the stop leg is capped separately by the load's bounce_stop_timeout_s. + # A leg that overruns its cap raises inside the bounce thread, failing + # the run with a correctly-attributed infra error (server too slow to + # stop / restart). Keep the restart cap well above a healthy restart + # (~6-40s on CI) so only a genuinely stuck boot trips it. + # + # The client-side budgets that must survive a bounce — close_drain and + # reconnect — are sized at runtime to exceed the whole stop+restart + # down-window (see _producer_loop, and the bounce wind-down in + # _run_fuzz). A healthy-but-slow bounce therefore can never strand a + # producer mid-drain: only a leg that overruns its own cap (attributed + # in the bounce thread) or a real client-side failure trips those + # budgets. That keeps a slow bounce from resurfacing as a misleading + # client close_drain timeout. BOUNCE_RESTART_TIMEOUT_SEC = 90 def setUp(self): @@ -2544,12 +2548,27 @@ def record_failure(msg: str): alter_thread.join(timeout=30) if bounce_thread is not None: - # Bounces are inherently slower (process restart + HTTP-up - # wait), so give the thread a generous wind-down budget. - bounce_thread.join(timeout=60) + # Once producers_done is set the thread starts no new bounce, + # but it finishes any in-flight one first. Size the wind-down + # to a whole cycle — stop() (≤ bounce_stop_timeout_s) + + # start() (≤ BOUNCE_RESTART_TIMEOUT_SEC), with headroom — so a + # healthy-but-slow last bounce completes before verification. + # join() returns the instant the thread exits, so this ceiling + # is only ever reached by a genuinely stuck lifecycle. + wind_down_sec = ( + fuzz.bounce_stop_timeout_s + + self.BOUNCE_RESTART_TIMEOUT_SEC + 30) + stop_event.set() + bounce_thread.join(timeout=wind_down_sec) if bounce_thread.is_alive(): - stop_event.set() - bounce_thread.join(timeout=60) + # A thread still mutating the fixture here would race the + # verification queries and teardown on shared _proc/_log + # state. Attribute it as the stuck lifecycle it is rather + # than let it resurface as a downstream query failure. + record_failure( + f'fuzz bounce: thread still alive ' + f'{wind_down_sec:.0f}s after producers finished; ' + f'server lifecycle is stuck') if bounce_thread.bounces_performed > 0: self._log( f'fuzz bounce summary: ' @@ -2582,11 +2601,19 @@ def record_failure(msg: str): def _producer_loop(self, sender_id, sf_root, load, fuzz, rnd, tables, next_ts, record_failure): - # `reconnect_max_duration_millis` is the explicit knob; the - # library auto-promotes `initial_connect_retry` to `sync` when - # any `reconnect_*` key is set, so a producer that races a - # bounce reuses the same 120s budget on its very first connect - # instead of getting one shot. + # Reconnect and close_drain share one budget, sized to outlast the + # worst-case single-bounce down-window: a full graceful stop() + # (bounded by the load's bounce_stop_timeout_s) plus a restart + # (bounded by BOUNCE_RESTART_TIMEOUT_SEC), with headroom. A + # healthy-but-slow bounce therefore never strands a producer + # mid-drain; a stop or restart that overruns its own cap fails in + # the bounce thread (correctly attributed), and only a real + # client-side failure trips this budget. The two knobs are kept + # equal on purpose: a reconnect that gave up earlier than the drain + # would fail the drain mid-window even though it had budget left. + client_budget_millis = int( + (fuzz.bounce_stop_timeout_s + self.BOUNCE_RESTART_TIMEOUT_SEC + 30) + * 1000) conf = self._sender_conf( sender_id, sf_root, @@ -2595,8 +2622,12 @@ def _producer_loop(self, sender_id, sf_root, load, fuzz, rnd, # connect, the producer that races the bounce fails fast with # an upgrade-response read error. `sync` makes the constructor # wait through the bounce up to reconnect_max_duration_millis. + # (The library auto-promotes initial_connect_retry to `sync` + # when any reconnect_* key is set, so the racing producer reuses + # this same budget on its very first connect instead of getting + # one shot.) initial_connect_retry='sync', - reconnect_max_duration_millis=120000, + reconnect_max_duration_millis=client_budget_millis, # The bounce tests restart the server faster than the reconnect # backoff cap can track, so the client keeps backing off and # misses the brief windows the server is up between restarts — @@ -2605,12 +2636,7 @@ def _producer_loop(self, sender_id, sf_root, load, fuzz, rnd, # the non-bounce tests, which never reconnect, so the backoff # never engages.) reconnect_max_backoff_millis=250, - # close_drain budget: long enough for SFA to replay queued - # frames into a freshly-restarted server. A bounce restart is - # capped below this (BOUNCE_RESTART_TIMEOUT_SEC) so a slow - # restart fails as an infra error instead of starving this - # drain. - close_flush_timeout_millis=self.CLOSE_FLUSH_TIMEOUT_SEC * 1000) + close_flush_timeout_millis=client_budget_millis) try: sender = self._connect_sender(conf) except Exception as e: # noqa: BLE001 From dd8f30ae02d1ef2602420bbc15b7be4701ae6eec Mon Sep 17 00:00:00 2001 From: Marko Topolnik Date: Wed, 24 Jun 2026 14:14:03 +0200 Subject: [PATCH 6/6] Unit-test readiness probe; fix docker stop float Two follow-ups from reviewing the min-HTTP readiness work. Add server-free unit tests for the readiness probe, which had no direct coverage. They pin the status-predicate asymmetry that the whole change hinges on (main /ping must answer 204 exactly, the min health endpoint counts any 2xx), the not-up-vs-fail-fast behaviour (a refused connection returns False so the retry loop continues, a dead process raises before any HTTP request), and start()'s selection of the main vs min check and its timeout. A silent flip of the 204-vs-2xx predicates is the most plausible future regression and now fails the suite. The tests drive QuestDbFixture with stand-in processes and a mocked urlopen, so no JVM or server is launched. Fix QuestDbDockerFixture.stop() passing a float to `docker stop -t`. The bounce path calls stop() with bounce_stop_timeout_s (a float, 120.0), and docker's integer -t flag rejects "120.0". Because the call runs with check=False, the failure was swallowed and the container force-removed, skipping the graceful grace period the bounce design relies on to drain WAL. Coerce to int. This is not reachable from the current fuzz test (it skips the docker fixture), but it is a latent foot-gun in the touched path. Co-Authored-By: Claude Opus 4.8 (1M context) --- system_test/fixture.py | 8 +- system_test/test_fixture_unit.py | 161 ++++++++++++++++++++++++++++++- 2 files changed, 167 insertions(+), 2 deletions(-) diff --git a/system_test/fixture.py b/system_test/fixture.py index fea86c16..48259f52 100644 --- a/system_test/fixture.py +++ b/system_test/fixture.py @@ -1069,7 +1069,13 @@ def stop(self, wait_timeout_sec=30): # `docker stop` sends SIGTERM (graceful: the JVM runs shutdown # hooks), then SIGKILLs after the grace period. Equivalent to # QuestDbFixture.stop()'s terminate()-then-kill(). - self._remove_container(['stop', '-t', str(wait_timeout_sec)]) + # + # docker's `-t` takes integer seconds; bounce callers pass a float + # budget (e.g. 120.0), which docker rejects ("invalid argument") — + # coerce to int so the graceful grace period is honoured rather + # than silently skipped, leaving only the immediate `rm -f`. + self._remove_container( + ['stop', '-t', str(int(wait_timeout_sec))]) def kill(self): # `docker kill` sends SIGKILL: an ungraceful crash with no diff --git a/system_test/test_fixture_unit.py b/system_test/test_fixture_unit.py index 576bee85..f8b087e8 100644 --- a/system_test/test_fixture_unit.py +++ b/system_test/test_fixture_unit.py @@ -36,7 +36,10 @@ being silently absorbed; * a clean shutdown stays quiet and does not raise; * `print_log()` reads the log as bytes so a force-kill that truncates - it mid-character can't crash the dump. + it mid-character can't crash the dump; +* the readiness probe (`_probe_http` and the main/min HTTP checks that + `start()` selects between) reports up/down without raising on a + not-yet-ready server and fails fast once the process has died. Run with:: @@ -54,6 +57,7 @@ import tempfile import time import unittest +from unittest import mock import fixture @@ -157,5 +161,160 @@ def test_handles_truncated_utf8_without_crashing(self): self.assertIn('BAD BYTES', dumped) +# Stand-ins for the probe tests: no real JVM or HTTP server is launched. +class _FakeProc: + """Minimal subprocess stand-in: poll() reports liveness.""" + + def __init__(self, alive=True): + self._alive = alive + + def poll(self): + # Popen.poll(): None while running, the exit code once dead. + return None if self._alive else 0 + + +class _FakeResponse: + """Minimal urlopen() result: the probe only reads .status.""" + + def __init__(self, status): + self.status = status + + +class ProbeHttpTest(unittest.TestCase): + """`_probe_http` is the shared readiness primitive: it asserts the + server is still alive, issues one GET, and maps the outcome to a + bool — never letting a not-yet-ready server's OSError abort the + enclosing retry loop.""" + + def _probe_fixture(self, tmp_dir, alive=True): + qdb = _make_fixture(tmp_dir) + qdb.http_server_port = 9000 + qdb.http_min_port = 9003 + qdb._proc = _FakeProc(alive=alive) + return qdb + + def test_accepted_status_returns_true(self): + with tempfile.TemporaryDirectory() as tmp_dir: + qdb = self._probe_fixture(tmp_dir) + with mock.patch('urllib.request.urlopen', + return_value=_FakeResponse(204)): + self.assertTrue( + qdb._probe_http(9000, '/ping', lambda s: s == 204)) + + def test_rejected_status_returns_false(self): + with tempfile.TemporaryDirectory() as tmp_dir: + qdb = self._probe_fixture(tmp_dir) + with mock.patch('urllib.request.urlopen', + return_value=_FakeResponse(503)): + self.assertFalse( + qdb._probe_http(9000, '/ping', lambda s: s == 204)) + + def test_connection_error_returns_false(self): + # A not-yet-ready server refuses the connection; the probe reports + # not-up rather than letting the OSError propagate and abort retry. + with tempfile.TemporaryDirectory() as tmp_dir: + qdb = self._probe_fixture(tmp_dir) + with mock.patch('urllib.request.urlopen', + side_effect=ConnectionRefusedError()): + self.assertFalse( + qdb._probe_http(9000, '/ping', lambda s: s == 204)) + + def test_dead_process_fails_fast_without_probing(self): + # _assert_server_alive runs first: a server that died during + # startup raises immediately instead of looping until the timeout, + # and no HTTP request is attempted. + with tempfile.TemporaryDirectory() as tmp_dir: + qdb = self._probe_fixture(tmp_dir, alive=False) + with mock.patch('urllib.request.urlopen') as urlopen: + with self.assertRaises(RuntimeError): + qdb._probe_http(9000, '/ping', lambda s: s == 204) + urlopen.assert_not_called() + + +class CheckHttpUpTest(unittest.TestCase): + """The two readiness checks must target distinct ports and paths and, + crucially, distinct status predicates: main `/ping` answers 204 + exactly, while the min health endpoint counts any 2xx as healthy.""" + + def _capture_probe(self, tmp_dir): + qdb = _make_fixture(tmp_dir) + qdb.http_server_port = 9000 + qdb.http_min_port = 9003 + calls = [] + qdb._probe_http = ( + lambda port, path, status_ok: + calls.append((port, path, status_ok)) or True) + return qdb, calls + + def test_main_probes_ping_requiring_exactly_204(self): + with tempfile.TemporaryDirectory() as tmp_dir: + qdb, calls = self._capture_probe(tmp_dir) + self.assertTrue(qdb._check_main_http_up()) + (port, path, status_ok), = calls + self.assertEqual(port, 9000) + self.assertEqual(path, '/ping') + self.assertTrue(status_ok(204)) + self.assertFalse(status_ok(200)) + self.assertFalse(status_ok(503)) + + def test_min_probes_status_accepting_any_2xx(self): + with tempfile.TemporaryDirectory() as tmp_dir: + qdb, calls = self._capture_probe(tmp_dir) + self.assertTrue(qdb._check_min_http_up()) + (port, path, status_ok), = calls + self.assertEqual(port, 9003) + self.assertEqual(path, '/status') + self.assertTrue(status_ok(200)) + self.assertTrue(status_ok(204)) + self.assertTrue(status_ok(299)) + self.assertFalse(status_ok(300)) + self.assertFalse(status_ok(404)) + + +class StartProbeSelectionTest(unittest.TestCase): + """start() picks its readiness check and timeout from its arguments: + the initial start gates on main `/ping` with the generous default, + while a bounce restart gates on the min endpoint with the tight cap + so a stuck boot fails fast instead of starving the producers.""" + + def _captured_start(self, tmp_dir, **start_kwargs): + qdb = _make_fixture(tmp_dir) + # Skip the post-start version query, which would hit real HTTP. + qdb._version_queried = True + captured = {} + + def fake_retry(predicate, timeout_sec=None, **_): + captured['predicate'] = predicate + captured['timeout_sec'] = timeout_sec + return True + + stderr = io.StringIO() + with contextlib.redirect_stderr(stderr), \ + mock.patch.object(fixture, 'retry', fake_retry), \ + mock.patch.object(fixture, '_find_java', return_value='java'), \ + mock.patch.object(fixture.subprocess, 'Popen', + return_value=_FakeProc(alive=True)), \ + mock.patch.object(fixture.atexit, 'register'): + qdb.start(**start_kwargs) + if qdb._log: + qdb._log.close() + return captured + + def test_initial_start_gates_on_main_http(self): + with tempfile.TemporaryDirectory() as tmp_dir: + captured = self._captured_start(tmp_dir) + self.assertEqual( + captured['predicate'].__name__, '_check_main_http_up') + self.assertEqual(captured['timeout_sec'], 300) + + def test_bounce_start_gates_on_min_http_with_tight_cap(self): + with tempfile.TemporaryDirectory() as tmp_dir: + captured = self._captured_start( + tmp_dir, start_timeout_sec=90, probe_min_http=True) + self.assertEqual( + captured['predicate'].__name__, '_check_min_http_up') + self.assertEqual(captured['timeout_sec'], 90) + + if __name__ == '__main__': unittest.main()