Skip to content

perf: Decouple metrics HTTP server into a dedicated child process #6855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ntkathole wants to merge 1 commit into
base: master
Choose a base branch
from
Open

perf: Decouple metrics HTTP server into a dedicated child process #6855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .secrets.baseline
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

112 changes: 95 additions & 17 deletions sdk/python/feast/metrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@
1. ``PROMETHEUS_MULTIPROCESS_DIR`` is set (to a temp dir if the user
has not already set it) **before** any metric objects are created.
2. Gauges specify ``multiprocess_mode`` so they aggregate correctly.
3. The metrics HTTP server uses ``MultiProcessCollector`` to read all
workers' metric files.
3. The metrics HTTP server runs in a **dedicated child process**
(not a thread) so that scrape aggregation never contends for the
GIL with request-serving workers. It uses ``MultiProcessCollector``
to read all workers' metric files.
4. Gunicorn hooks (``post_worker_init``, ``child_exit``) are wired up
in ``feature_server.py`` to start per-worker monitoring and to
clean up dead-worker files.
Expand All @@ -44,6 +46,7 @@
import atexit
import json
import logging
import multiprocessing
import os
import shutil
import tempfile
Expand Down Expand Up @@ -583,6 +586,60 @@ def init_worker_freshness_monitoring(store: "FeatureStore"):
t.start()


def _run_metrics_server(port: int, mp_dir: str, ready_event=None):
"""Entry point for the dedicated metrics-server child process.

Runs in its own process so that Prometheus scrape aggregation
(``MultiProcessCollector`` file reads + text serialization) never
contends for the GIL with request-serving workers or the Gunicorn
master.

The function re-imports ``prometheus_client`` so it works correctly
on platforms that use the ``spawn`` multiprocessing start method
(e.g. macOS with Python 3.8+).

Args:
port: TCP port for the Prometheus HTTP endpoint.
mp_dir: Path to the ``PROMETHEUS_MULTIPROCESS_DIR``.
ready_event: Optional ``multiprocessing.Event`` that is set once
the server has successfully bound to *port*. The parent
process can wait on this to confirm startup succeeded.
"""
import signal

os.environ.setdefault("PROMETHEUS_MULTIPROCESS_DIR", mp_dir)
os.environ.setdefault("PROMETHEUS_MULTIPROC_DIR", mp_dir)
Comment thread
ntkathole marked this conversation as resolved.

from wsgiref.simple_server import make_server

Comment thread
ntkathole marked this conversation as resolved.
from prometheus_client import CollectorRegistry, make_wsgi_app
from prometheus_client.multiprocess import MultiProcessCollector

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 "
"(multiprocess dir: %s)",
port,
mp_dir,
)
return

if ready_event is not None:
ready_event.set()

def _shutdown(signum, frame):
threading.Thread(target=httpd.shutdown, daemon=True).start()

signal.signal(signal.SIGTERM, _shutdown)
signal.signal(signal.SIGINT, _shutdown)

httpd.serve_forever()


def start_metrics_server(
store: "FeatureStore",
port: int = 8000,
Expand All @@ -593,8 +650,14 @@ def start_metrics_server(
"""
Start the Prometheus metrics HTTP server and background monitoring threads.

Uses ``MultiProcessCollector`` so that metrics from all Gunicorn
workers are correctly aggregated when Prometheus scrapes port *port*.
The HTTP endpoint runs in a **dedicated child process** so that
scrape-time aggregation (reading mmap files from every Gunicorn
worker, merging, and serializing to Prometheus text format) is
fully isolated from the request-serving GIL.

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.

Args:
store: The FeatureStore instance (used for freshness checks).
Expand Down Expand Up @@ -627,20 +690,35 @@ def start_metrics_server(
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()
logger.info(
"Prometheus metrics server started on port %d (multiprocess-safe)", port
ready_event = multiprocessing.Event()
metrics_proc = multiprocessing.Process(
>
args=(port, _prometheus_mp_dir, ready_event),
daemon=True,
name="feast-metrics-server",
)
metrics_proc.start()

if ready_event.wait(timeout=5):
logger.info(
"Prometheus metrics server started on port %d in dedicated process (pid=%d)",
port,
metrics_proc.pid,
)
else:
if not metrics_proc.is_alive():
logger.error(
"Prometheus metrics server process exited before becoming ready "
"(port=%d). Check logs for initialization errors.",
port,
)
else:
logger.warning(
"Prometheus metrics server process (pid=%d) did not signal readiness "
"within 5 s — it may still be starting (port=%d).",
metrics_proc.pid,
port,
)

if _config.resource and start_resource_monitoring:
resource_thread = threading.Thread(
Expand Down
Loading
Loading