Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 46 additions & 19 deletions system_test/fixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -528,10 +533,15 @@ 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 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 ''
Expand All @@ -552,7 +562,9 @@ def start(self):
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}
Expand Down Expand Up @@ -623,13 +635,18 @@ def start(self):
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
Expand All @@ -640,8 +657,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.')
Comment on lines 700 to +704

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Clean up the managed JVM when readiness times out.

The new bounded retry(..., timeout_sec=start_timeout_sec) makes start failures an expected infra signal, but the managed fixture still re-raises without stopping the spawned process or closing the log. Docker already calls self.stop() on the same path; mirror that here so failed initial starts do not leak a JVM/ports.

Proposed fix
         except:
             sys.stderr.write(f'QuestDB log at `{self._log_path}`:\n')
             self.print_log()
+            try:
+                self.stop()
+            except Exception as cleanup_error:
+                sys.stderr.write(
+                    'Failed to clean up QuestDB after start failure: '
+                    f'{type(cleanup_error).__name__}: {cleanup_error}\n')
             raise
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@system_test/fixture.py` around lines 700 - 704, The readiness timeout path in
the managed JVM startup flow currently re-raises without cleaning up the spawned
process or log handle, which can leak resources on failed starts. Update the
startup logic around the retry call in the managed fixture (the same flow that
uses self.stop() for Docker) so that when startup readiness times out, it
explicitly stops the JVM/process and closes any open log before re-raising. Use
the existing startup/cleanup symbols in this fixture to keep the failure path
consistent with other managed resources.

except:
sys.stderr.write(f'QuestDB log at `{self._log_path}`:\n')
self.print_log()
Expand All @@ -656,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
Expand Down Expand Up @@ -837,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
Expand All @@ -857,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',
Expand All @@ -884,18 +905,20 @@ 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
(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()

cmd = ['run', '-d', '--name', _unique_container_name('qdb_systest')]
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']
Expand Down Expand Up @@ -924,8 +947,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()
Expand Down Expand Up @@ -953,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
Expand Down
28 changes: 20 additions & 8 deletions system_test/qwp_ws_fuzz.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
25 changes: 21 additions & 4 deletions system_test/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Loading