FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: Do not log /metrics requests from the pre-fork metrics server thread (Gunicorn deadlock) by aborgatin · Pull Request #6929 · feast-dev/feast · GitHub

Repository navigation

fix: Do not log /metrics requests from the pre-fork metrics server thread (Gunicorn deadlock) - #6929

Merged
ntkathole merged 1 commit into
feast-dev:masterfrom
aborgatin:fix/metrics-server-quiet-handler
Oct 7, 2026
Merged

ntkathole merged 1 commit into
feast-dev:masterfrom
aborgatin:fix/metrics-server-quiet-handler

Conversation

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

The metrics HTTP server runs as a thread in the Gunicorn master, which forks the workers. Its default WSGIRequestHandler writes an access-log line to stderr for every scrape. A fork while that thread holds the stderr buffer lock leaves the new worker with the lock held forever: the worker blocks on its first log line ("Booting worker") and never serves a request. Details, stacks and a reproduction in #6928 (same class of problem as #6647).

This adds _QuietWSGIRequestHandler (no-op log_message, like prometheus_client's own _SilentHandler) and uses it for both the IPv4 and the dual-stack server built by _make_metrics_httpd. Scrapes are no longer written to stderr; nothing else changes.

Tested on Kubernetes with --max-requests 20 and a scrape storm: 5 hangs in 318 forks before, 0 hangs in 2,000 forks after. Locally (steps in #6928): a hang after 12–29 forks before, none in 336 forks after.

Which issue(s) this PR fixes:

Fixes #6928

Checks

  • I've made sure the tests are passing. (pytest sdk/python/tests/unit/test_metrics.py: 117 passed; ruff check and ruff format --check pass; mypy feast reports no errors in metrics.py)
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests – TestMetricsHttpdDoesNotLog: a request to the server built by _make_metrics_httpd writes nothing to stderr (fails before the change, passes after)
  • Integration tests

🤖 Generated with Claude Code

aborgatin requested a review from a team as a code owner October 2, 2026 10:09

codecov-commenter commented Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.06%. Comparing base (8cfe891) to head (8e0f1e4).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #6929   +/-   ##
=======================================
  Coverage   49.06%   49.06%           
=======================================
  Files         433      433           
  Lines       54314    54316    +2     
  Branches     7912     7912           
=======================================
+ Hits        26647    26649    +2     
  Misses      25788    25788           
  Partials     1879     1879           
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 50.45% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/metrics.py 81.81% <100.00%> (+0.17%) ⬆️

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 8cfe891...8e0f1e4. 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.

aborgatin force-pushed the fix/metrics-server-quiet-handler branch from 76d2f7b to 8e0f1e4 Compare October 6, 2026 07:52

Copy link
Copy Markdown
Contributor Author

@jyejare rebased onto master to pick up the pixi fix (#6947) – could you approve the workflow runs? Thanks!

…read

The metrics HTTP server runs as a thread in the Gunicorn master, which
forks the workers. Its default WSGIRequestHandler writes an access-log
line to stderr for every scrape. A fork while that thread holds the
stderr buffer lock leaves the new worker with the lock held forever: it
blocks on its first log line ("Booting worker") and never serves.

Use a request handler with a no-op log_message for both the IPv4 and
the dual-stack server built by _make_metrics_httpd, like
prometheus_client's own _SilentHandler.

Fixes feast-dev#6928

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Alexandr Borgatin <a.borgatin@yandex.ru>
ntkathole force-pushed the fix/metrics-server-quiet-handler branch from 8e0f1e4 to 9237fb0 Compare October 7, 2026 04:49
ntkathole merged commit 7087f1f into feast-dev:master Oct 7, 2026
2 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feast serve: Gunicorn worker deadlocks on startup when forked during a /metrics request (follow-up to #6647)

5 participants


Back | FazBrowse Home | New Git URL