Skip to content

fix: Defer feature-freshness thread to post-fork to avoid Gunicorn deadlock - #6648

Merged
jyejare merged 1 commit into
feast-dev:masterfrom
casaar97:fix/freshness-thread-postfork-deadlock
Jul 27, 2026
Merged

fix: Defer feature-freshness thread to post-fork to avoid Gunicorn deadlock#6648
jyejare merged 1 commit into
feast-dev:masterfrom
casaar97:fix/freshness-thread-postfork-deadlock

Conversation

@casaar97

Copy link
Copy Markdown
Contributor

Fixes #6647

Summary

feast_metrics.start_metrics_server() starts a feature-freshness thread in the Gunicorn master process, before Gunicorn forks its worker(s). That thread's first action, with no delay, is update_feature_freshness()store.list_feature_views(), which lazily builds the registry for the first time. For registry backends needing a lazy DBAPI import (e.g. the SQL registry importing pymysql via SQLAlchemy's create_engine()), this means a thread in the master can be mid-import, holding CPython's per-module import lock, at the exact moment Gunicorn forks a worker.

POSIX fork() only duplicates the calling thread into the child process; every other thread in the parent, including this one, simply ceases to exist in the worker. If the fork lands while that thread holds a module's import lock, the lock stays permanently held in the new worker, since there is no longer any thread that can finish the import and release it. The worker's own later attempt to build its registry then deadlocks forever with no error - the process just hangs at "Waiting for application startup.". This is intermittent by nature: it only manifests if the fork lands inside that narrow timing window (see #6647 for the full write-up, including a live py-spy stack trace confirming the exact mechanism).

Resource monitoring already avoids this correctly (start_resource_monitoring=not uses_gunicorn plus the post_worker_init hook calling init_worker_monitoring()), but the freshness thread wasn't given the same treatment. This PR applies the identical, already-established pattern:

  • start_metrics_server() gains a start_freshness_monitoring flag, deferred exactly like start_resource_monitoring already is.
  • A new init_worker_freshness_monitoring(store) mirrors init_worker_monitoring().
  • FeastServeApplication's post_worker_init hook now also calls init_worker_freshness_monitoring(store) after the fork (via a closure capturing store, since the freshness thread needs the store reference, unlike resource monitoring).

Test plan

  • Added TestInitWorkerFreshnessMonitoring covering that the new function starts/doesn't start the thread based on the freshness config flag, mirroring the existing init_worker_monitoring behavior.
  • Added test_freshness_monitoring_deferred_same_as_resource_monitoring to TestMetricsYamlConfig, asserting start_freshness_monitoring is always passed identically to start_resource_monitoring from start_server() - a regression guard so this can't silently drift out of sync again.
  • sdk/python/tests/unit/test_metrics.py (89 tests) and sdk/python/tests/unit/test_feature_server.py / test_feature_server_utils.py (92 tests) pass locally.
  • ruff check / ruff format --check pass on all changed files.

@casaar97
casaar97 requested a review from a team as a code owner July 27, 2026 13:27
@casaar97
casaar97 force-pushed the fix/freshness-thread-postfork-deadlock branch from bb3deed to 597a845 Compare July 27, 2026 13:30
@ntkathole

Copy link
Copy Markdown
Member

@jyejare ^

@codecov-commenter

codecov-commenter commented Jul 27, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.93%. Comparing base (f9923bc) to head (5376751).

Files with missing lines Patch % Lines
sdk/python/feast/feature_server.py 60.00% 2 Missing ⚠️
sdk/python/feast/metrics.py 80.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6648   +/-   ##
=======================================
  Coverage   45.93%   45.93%           
=======================================
  Files         414      414           
  Lines       49999    50006    +7     
  Branches     7146     7147    +1     
=======================================
+ Hits        22965    22972    +7     
  Misses      25423    25423           
  Partials     1611     1611           
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 47.20% <70.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/metrics.py 75.51% <80.00%> (+0.51%) ⬆️
sdk/python/feast/feature_server.py 61.85% <60.00%> (+0.18%) ⬆️

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 f9923bc...5376751. 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.

…adlock

feast_metrics.start_metrics_server() starts a feature-freshness thread
in the Gunicorn master process, before Gunicorn forks its worker(s).
That thread's first action, with no delay, is update_feature_freshness()
-> store.list_feature_views(), which lazily builds the registry for the
first time. For registry backends needing a lazy DBAPI import (e.g. the
SQL registry importing pymysql via SQLAlchemy's create_engine()), this
means a thread in the master can be mid-import, holding CPython's
per-module import lock, at the exact moment Gunicorn forks a worker.

POSIX fork() only duplicates the calling thread into the child process;
every other thread in the parent, including this one, simply ceases to
exist in the worker. If the fork lands while that thread holds a
module's import lock, the lock stays permanently held in the new
worker, since there is no longer any thread that can finish the import
and release it. The worker's own later attempt to build its registry
then deadlocks forever with no error - the process just hangs at
"Waiting for application startup." This is intermittent by nature: it
only manifests if the fork lands inside that narrow timing window.

Resource monitoring already avoids this correctly (start_resource_monitoring=
not uses_gunicorn plus the post_worker_init hook calling
init_worker_monitoring()), but the freshness thread was not given the
same treatment. This applies the identical pattern: start_metrics_server
gains a start_freshness_monitoring flag (deferred exactly like resource
monitoring's), and FeastServeApplication's post_worker_init hook now
also calls the new init_worker_freshness_monitoring(store) after the
fork, instead of feast_metrics.py starting it unconditionally beforehand.

Fixes feast-dev#6647

Signed-off-by: Carlos Sánchez <carlos.sancheza@cabify.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an intermittent feast serve startup deadlock under Gunicorn by deferring the feature-freshness monitoring thread until after workers are forked, mirroring the existing “post-fork” pattern already used for resource monitoring.

Changes:

  • Add a start_freshness_monitoring flag to feast.metrics.start_metrics_server() to prevent starting the freshness thread in the Gunicorn master process.
  • Introduce init_worker_freshness_monitoring(store) and invoke it from Gunicorn’s post_worker_init hook (via a functools.partial capturing the FeatureStore).
  • Add unit/regression tests to ensure the worker-init freshness behavior is gated by config and stays in lockstep with resource monitoring deferral.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
sdk/python/feast/metrics.py Adds post-fork freshness init helper and defers freshness thread startup via a new start_freshness_monitoring flag.
sdk/python/feast/feature_server.py Updates Gunicorn post_worker_init hook wiring to start freshness monitoring per worker after fork.
sdk/python/tests/unit/test_metrics.py Adds tests for init_worker_freshness_monitoring gating and ensures deferral flags remain synchronized.
sdk/python/tests/unit/test_feature_server.py Adds a Gunicorn hook test asserting both resource and freshness monitoring are started post-fork.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jyejare
jyejare merged commit 104ad10 into feast-dev:master Jul 27, 2026
27 checks passed
jyejare added a commit to opendatahub-io/feast that referenced this pull request Jul 28, 2026
…#166)

* feat: Multi-arch publish for feast operator image

Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

* feat: Allow users to have protected project on shared registry

Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>

* fix: Defer feature-freshness thread to post-fork to avoid Gunicorn deadlock (feast-dev#6648)

feast_metrics.start_metrics_server() starts a feature-freshness thread
in the Gunicorn master process, before Gunicorn forks its worker(s).
That thread's first action, with no delay, is update_feature_freshness()
-> store.list_feature_views(), which lazily builds the registry for the
first time. For registry backends needing a lazy DBAPI import (e.g. the
SQL registry importing pymysql via SQLAlchemy's create_engine()), this
means a thread in the master can be mid-import, holding CPython's
per-module import lock, at the exact moment Gunicorn forks a worker.

POSIX fork() only duplicates the calling thread into the child process;
every other thread in the parent, including this one, simply ceases to
exist in the worker. If the fork lands while that thread holds a
module's import lock, the lock stays permanently held in the new
worker, since there is no longer any thread that can finish the import
and release it. The worker's own later attempt to build its registry
then deadlocks forever with no error - the process just hangs at
"Waiting for application startup." This is intermittent by nature: it
only manifests if the fork lands inside that narrow timing window.

Resource monitoring already avoids this correctly (start_resource_monitoring=
not uses_gunicorn plus the post_worker_init hook calling
init_worker_monitoring()), but the freshness thread was not given the
same treatment. This applies the identical pattern: start_metrics_server
gains a start_freshness_monitoring flag (deferred exactly like resource
monitoring's), and FeastServeApplication's post_worker_init hook now
also calls the new init_worker_freshness_monitoring(store) after the
fork, instead of feast_metrics.py starting it unconditionally beforehand.

Fixes feast-dev#6647

Signed-off-by: Carlos Sánchez <carlos.sancheza@cabify.com>
Co-authored-by: Carlos Sánchez <carlos.sancheza@cabify.com>

---------

Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
Signed-off-by: Carlos Sánchez <carlos.sancheza@cabify.com>
Co-authored-by: ntkathole <nikhilkathole2683@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Carlos Sánchez <i52saarc@uco.es>
Co-authored-by: Carlos Sánchez <carlos.sancheza@cabify.com>
aniketpalu pushed a commit to aniketpalu/feast that referenced this pull request Jul 28, 2026
…adlock (feast-dev#6648)

feast_metrics.start_metrics_server() starts a feature-freshness thread
in the Gunicorn master process, before Gunicorn forks its worker(s).
That thread's first action, with no delay, is update_feature_freshness()
-> store.list_feature_views(), which lazily builds the registry for the
first time. For registry backends needing a lazy DBAPI import (e.g. the
SQL registry importing pymysql via SQLAlchemy's create_engine()), this
means a thread in the master can be mid-import, holding CPython's
per-module import lock, at the exact moment Gunicorn forks a worker.

POSIX fork() only duplicates the calling thread into the child process;
every other thread in the parent, including this one, simply ceases to
exist in the worker. If the fork lands while that thread holds a
module's import lock, the lock stays permanently held in the new
worker, since there is no longer any thread that can finish the import
and release it. The worker's own later attempt to build its registry
then deadlocks forever with no error - the process just hangs at
"Waiting for application startup." This is intermittent by nature: it
only manifests if the fork lands inside that narrow timing window.

Resource monitoring already avoids this correctly (start_resource_monitoring=
not uses_gunicorn plus the post_worker_init hook calling
init_worker_monitoring()), but the freshness thread was not given the
same treatment. This applies the identical pattern: start_metrics_server
gains a start_freshness_monitoring flag (deferred exactly like resource
monitoring's), and FeastServeApplication's post_worker_init hook now
also calls the new init_worker_freshness_monitoring(store) after the
fork, instead of feast_metrics.py starting it unconditionally beforehand.

Fixes feast-dev#6647

Signed-off-by: Carlos Sánchez <carlos.sancheza@cabify.com>
Co-authored-by: Carlos Sánchez <carlos.sancheza@cabify.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Intermittent deadlock on feast serve startup under gunicorn: feature-freshness thread starts pre-fork and can freeze an import lock forever

5 participants