perf: Decouple metrics HTTP server into a dedicated child process

[ntkathole]

Summary

Replace the daemon thread that serves the Prometheus /metrics endpoint with a multiprocessing.Process so that scrape-time aggregation (MultiProcessCollector reads + text serialization) runs with its own GIL, fully isolated from request-serving workers and the Gunicorn master.

Problem

When Prometheus scrapes the /metrics endpoint, the current daemon thread performs:

1. File listing of all .db files in PROMETHEUS_MULTIPROCESS_DIR
2. Reading mmap-backed metric files from every Gunicorn worker
3. Aggregating/merging metrics across workers
4. Serializing to Prometheus text format

All of this runs under the same GIL as the Gunicorn master process. With high metric cardinality (many label combinations), this scrape overhead can cause GIL contention that indirectly affects request latency.

Solution

Move the WSGI HTTP metrics server into a dedicated child process via multiprocessing.Process(daemon=True):

• _run_metrics_server() — new top-level function that serves as the child process entry point. It re-imports prometheus_client for macOS spawn safety and registers SIGTERM/SIGINT handlers for graceful shutdown.
• start_metrics_server() — now spawns a Process instead of a Thread. The function signature and call sites are unchanged.
• Background monitoring threads (resource, freshness) remain in-process because they only write to mmap-backed Gauges — an operation that is fast and does not benefit from process isolation.

Why this is safe

Aspect	Before (thread)	After (process)
Metrics port	:8000 in master process	:8000 in child process
GIL isolation	Shared with master	Fully isolated
daemon=True	Thread auto-joins on exit	Process auto-terminates on parent exit
Prometheus scrape	Same metrics, same format	Identical — MultiProcessCollector reads same mmap files
Kubernetes/HPA	Transparent	Transparent — same port, same pod network namespace
spawn safety	N/A	_run_metrics_server is top-level, re-imports deps

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 22.22222% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.63%. Comparing base (d9ea120) to head (755e967).

Files with missing lines	Patch %	Lines
sdk/python/feast/metrics.py	22.22%	14 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6855      +/-   ##
==========================================
+ Coverage   47.62%   47.63%   +0.01%     
==========================================
  Files         422      422              
  Lines       52352    52362      +10     
  Branches     7596     7596              
==========================================
+ Hits        24931    24944      +13     
+ Misses      25646    25642       -4     
- Partials     1775     1776       +1     

Flag	Coverage Δ
go-feature-server	30.58% <ø> (ø)
python-unit	48.97% <22.22%> (+0.01%)	⬆️

Files with missing lines	Coverage Δ
sdk/python/feast/metrics.py	78.64% <22.22%> (+3.13%)	⬆️

... and 1 file with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9ea120...755e967. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jyejare]

The PR cleanly isolates Prometheus scrape aggregation in a dedicated process and preserves the existing in-process monitoring behavior. The documentation and unit tests cover the process construction and monitoring flags well, but the child shutdown path can deadlock because BaseServer.shutdown() is called from the same thread running serve_forever(). Startup failures and lifecycle management are also not surfaced to the parent, which can leave the service reporting metrics as started when the endpoint is unavailable.

[jyejare]

[Critical] Avoid calling httpd.shutdown() from the serve_forever thread

wsgiref's BaseServer.shutdown() is documented to be called from a thread other than the one running serve_forever(); otherwise it waits for serve_forever() to exit while serve_forever() is blocked waiting for shutdown(), causing a deadlock. Since the signal handler runs in the child process's main thread, SIGTERM/SIGINT can leave the metrics process alive and prevent graceful shutdown. Invoke shutdown from a separate helper thread, or set a stop flag and exit the serving loop without calling shutdown synchronously from the signal handler.

Suggested:

Suggested change

	registry = CollectorRegistry()
	MultiProcessCollector(registry)
	httpd = make_server("", port, make_wsgi_app(registry))
	+ def _shutdown(signum, frame):
	+ threading.Thread(target=httpd.shutdown, daemon=True).start()
	+
	+ signal.signal(signal.SIGTERM, _shutdown)
	+ signal.signal(signal.SIGINT, _shutdown)

[jyejare]

[Warning] Surface child-process startup failures

The parent logs that the metrics server started immediately after Process.start(), but the child may fail while importing dependencies or binding the port. In that case callers receive a false success signal and there is no monitoring or error propagation for the dead child. Add a startup handshake (for example, a pipe or Event set after make_server succeeds), and either wait briefly for readiness or log/report the child exception when startup fails.

Suggested:

Suggested change

	os.environ.setdefault("PROMETHEUS_MULTIPROCESS_DIR", mp_dir)
	os.environ.setdefault("PROMETHEUS_MULTIPROC_DIR", mp_dir)
	from wsgiref.simple_server import make_server
	+ try:
	+ registry = CollectorRegistry()
	+ MultiProcessCollector(registry)
	+ httpd = make_server("", port, make_wsgi_app(registry))
	+ ready_event.set()
	+ except Exception:
	+ logger.exception("Failed to start Prometheus metrics server on port %d", port)
	+ raise

[jyejare]

[Warning] Manage the metrics process lifecycle explicitly

The Process handle is local and is not returned, stored, or given an explicit shutdown path. Relying solely on daemon-process semantics means normal application shutdown may terminate the HTTP server abruptly, and callers cannot detect or clean up a failed/stale child. Retain the handle in module state or return it, register an atexit shutdown/join callback, and ensure repeated starts do not create orphaned servers.

Suggested:

Suggested change

	audit_logging=False,
	)
	from prometheus_client import CollectorRegistry, make_wsgi_app
	from prometheus_client.multiprocess import MultiProcessCollector
	registry = CollectorRegistry()
	MultiProcessCollector(registry)
	from wsgiref.simple_server import make_server
	httpd = make_server("", port, make_wsgi_app(registry))
	metrics_thread = threading.Thread(target=httpd.serve_forever, daemon=True)
	metrics_thread.start()
	metrics_proc = multiprocessing.Process(
	target=_run_metrics_server,
	args=(port, _prometheus_mp_dir),
	daemon=True,
	name="feast-metrics-server",
	)
	metrics_proc.start()
	logger.info(
	+ metrics_proc = multiprocessing.Process(
	+ target=_run_metrics_server,
	+ args=(port, _prometheus_mp_dir),
	+ daemon=True,
	+ name="feast-metrics-server",
	+ )
	+ metrics_proc.start()
	+ _metrics_processes.append(metrics_proc)
	+ atexit.register(_stop_metrics_process, metrics_proc)

[jyejare]

[Warning] Handle bind and initialization exceptions in the child

make_server() and make_wsgi_app() can raise, most notably when the configured port is already in use or the metrics directory is inaccessible. The exception currently only terminates the child process, while the parent has already emitted a success log. Wrap initialization in an exception handler that reports the failure through a startup channel or at minimum emits a clear child-process log including the port and directory.

Suggested:

Suggested change

	os.environ.setdefault("PROMETHEUS_MULTIPROCESS_DIR", mp_dir)
	os.environ.setdefault("PROMETHEUS_MULTIPROC_DIR", mp_dir)
	+ try:
	+ registry = CollectorRegistry()
	+ MultiProcessCollector(registry)
	+ httpd = make_server("", port, make_wsgi_app(registry))
	+ except Exception:
	+ logger.exception(
	+ "Failed to initialize Prometheus metrics server on port %d", port
	+ )
	+ raise

[jyejare]

[Warning] Add tests for child server shutdown and startup failure

The tests mock multiprocessing.Process and verify constructor arguments, but they never execute _run_metrics_server. Consequently, the SIGTERM deadlock, incorrect signal handling, and bind/import failure behavior are untested—the highest-risk parts of this change. Add focused tests with a fake HTTP server to assert that shutdown is invoked asynchronously or that the child exits cleanly, plus a test that a make_server failure is reported through the startup mechanism.

Suggested:

Suggested change

	def test_launches_dedicated_process(self):
	"""start_metrics_server spawns a multiprocessing.Process, not a Thread."""
	import feast.metrics as m
	mock_store = MagicMock()
	with (
	patch("feast.metrics.multiprocessing.Process") as mock_proc_cls,
	patch("feast.metrics.threading.Thread"),
	):
	mock_proc = MagicMock()
	mock_proc.pid = 12345
	mock_proc_cls.return_value = mock_proc
	m.start_metrics_server(mock_store, port=9090)
	mock_proc_cls.assert_called_once_with(
	target=m._run_metrics_server,
	args=(9090, m._prometheus_mp_dir),
	daemon=True,
	name="feast-metrics-server",
	)
	mock_proc.start.assert_called_once()
	def test_process_receives_correct_mp_dir(self):
	"""The child process receives the multiprocess directory path."""
	import feast.metrics as m
	mock_store = MagicMock()
	with (
	patch("feast.metrics.multiprocessing.Process") as mock_proc_cls,
	patch("feast.metrics.threading.Thread"),
	):
	mock_proc = MagicMock()
	mock_proc.pid = 99
	mock_proc_cls.return_value = mock_proc
	m.start_metrics_server(mock_store)
	_, kwargs = mock_proc_cls.call_args
	assert kwargs["args"][1] == m._prometheus_mp_dir
	def test_background_threads_still_started(self):
	"""Resource and freshness monitoring threads still launch in-process."""
	import feast.metrics as m
	mock_store = MagicMock()
	flags = m._MetricsFlags(
	enabled=True,
	resource=True,
	freshness=True,
	request=True,
	online_features=True,
	push=True,
	materialization=True,
	)
	with (
	patch("feast.metrics.multiprocessing.Process") as mock_proc_cls,
	patch("feast.metrics.threading.Thread") as mock_thread_cls,
	):
	mock_proc = MagicMock()
	mock_proc.pid = 1
	mock_proc_cls.return_value = mock_proc
	m.start_metrics_server(
	mock_store,
	metrics_config=flags,
	start_resource_monitoring=True,
	start_freshness_monitoring=True,
	)
	thread_calls = mock_thread_cls.call_args_list
	targets = [c.kwargs.get("target") for c in thread_calls]
	assert m.monitor_resources in targets
	assert m.monitor_freshness in targets
	def test_no_threads_when_monitoring_deferred(self):
	"""When monitoring is deferred to workers, no threads are started."""
	import feast.metrics as m
	mock_store = MagicMock()
	with (
	patch("feast.metrics.multiprocessing.Process") as mock_proc_cls,
	patch("feast.metrics.threading.Thread") as mock_thread_cls,
	):
	mock_proc = MagicMock()
	mock_proc.pid = 1
	mock_proc_cls.return_value = mock_proc
	m.start_metrics_server(
	mock_store,
	start_resource_monitoring=False,
	start_freshness_monitoring=False,
	)
	mock_thread_cls.assert_not_called()
	class TestMetricsYamlConfig:
	"""Verify metrics config in feature_store.yaml is respected.
	+ def test_child_handles_sigterm_without_deadlocking(self):
	+ """The child shutdown handler must not synchronously deadlock serve_forever."""
	+ # Patch make_server with a fake server and invoke the registered handler.
	+ # Assert shutdown is dispatched asynchronously and the serving loop exits.