Skip to content

Comments

fix: Unstable PROMETHEUS_MULTIPROC_DIR#116

Merged
matthewelwell merged 4 commits intomainfrom
fix/stable-multiproc-dir-clean-up
Nov 7, 2025
Merged

fix: Unstable PROMETHEUS_MULTIPROC_DIR#116
matthewelwell merged 4 commits intomainfrom
fix/stable-multiproc-dir-clean-up

Conversation

@khvn26
Copy link
Member

@khvn26 khvn26 commented Nov 5, 2025

In this PR, we change the following:

  1. Use a default stable path for PROMETHEUS_MULTIPROC_DIR instead of relying on a random one offered by tempfile.mkdtemp to completely avoid new directories spam reported in temporary directories are not removed flagsmith#5990.
  2. Move the cleanup to app startup. This ensures that the directory exists throughout Gunicorn's worker lifecycle.

@khvn26 khvn26 requested a review from a team as a code owner November 5, 2025 12:10
@khvn26 khvn26 requested review from emyller and removed request for a team November 5, 2025 12:10
@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.63%. Comparing base (00a973a) to head (6bf2ea8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
- Coverage   94.67%   94.63%   -0.05%     
==========================================
  Files          76       77       +1     
  Lines        2479     2459      -20     
==========================================
- Hits         2347     2327      -20     
  Misses        132      132              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

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

I don't have context yet, but I'm interested in possibly cleaning up temporary files on startup (just in case the previous run ended unexpectedly), and on shutdown (because it's polite to cleanup resources after use).

Unless there's reason I don't know, the better approach to manage temporary files should be via tempfile as long as we always cleanup.

@khvn26
Copy link
Member Author

khvn26 commented Nov 7, 2025

and on shutdown (because it's polite to cleanup resources after use).

I agree, but see below.

Unless there's reason I don't know, the better approach to manage temporary files should be via tempfile as long as we always cleanup.

tempfile.mkdtemp does not guarantee cleanup, and results in empty dir spam in some environments — see original issue Flagsmith/flagsmith#5990 for details.

temfile.TemporaryDirectory was supposed to work as we'd expect, but apparently cleans up too eagerly for Gunicorn's fork model (perhaps due to weakref being used) — see https://flagsmith.sentry.io/issues/6994120980/events/a31de60ca80e4ea08dda39669e9b02c5/. The issue is what this PR is fixing.

@khvn26 khvn26 requested a review from emyller November 7, 2025 11:37
emyller
emyller previously approved these changes Nov 7, 2025
Copy link
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

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

LGTM!

@matthewelwell matthewelwell merged commit b319fd4 into main Nov 7, 2025
3 checks passed
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.

4 participants