prometheus / prometheus/client_python

MultiProcessCollector fails permanently on an empty metrics file left by a killed worker

オープン
#1,199 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

主要言語
Python
スター
4.4k
フォーク
876
平均マージ
8日 4時間
マージ済み PR(30日)
1

説明

What did you do?

Ran a FastAPI app under gunicorn with PROMETHEUS_MULTIPROC_DIR set and several workers, and scraped /metrics.

What did you expect to see?

A successful scrape. Failing that, a failure confined to the worker whose file could not be read.

What did you see instead?

MultiProcessCollector.collect() raising, and taking every metric in the registry with it:

struct.error: unpack_from requires a buffer of at least 4 bytes for unpacking 4 bytes at offset 0 (actual buffer size is 0)

  File ".../prometheus_client/mmap_dict.py", line 90, in read_all_values_from_file
    used = _unpack_integer(data, 0)[0]
  File ".../prometheus_client/multiprocess.py", line 63, in _read_metrics
    file_values = MmapedDict.read_all_values_from_file(f)
  File ".../prometheus_client/multiprocess.py", line 43, in merge
    metrics = MultiProcessCollector._read_metrics(files)
  File ".../prometheus_client/multiprocess.py", line 158, in collect
    return self.merge(files, accumulate=True)

(Line numbers are from 0.20.0, where this was hit in production; collect is at 171 in 0.26.0. The relevant code is byte-identical between the two.)

Mechanism

MmapedDict.__init__ creates the backing file and only sizes it afterwards, so there is a window in which it exists at 0 bytes:

self._f = open(filename, 'rb' if read_mode else 'a+b')   # creates at 0 bytes
capacity = os.fstat(self._f.fileno()).st_size
if capacity == 0:
    self._f.truncate(_INITIAL_MMAP_SIZE)                 # sized only here

read_all_values_from_file has no guard for that, so _unpack_integer(data, 0) on an empty read raises. This is at least one verified mechanism; a truncate failing on a full volume (ENOSPC/EDQUOT), or external cleanup that truncates rather than unlinks, would leave the same state.

This has two quite different consequences, and the second is the reason I'm filing:

1. Transient. A scrape lands inside the creation window. Verified with 6 concurrent writer processes and a reader looping glob+read using a verbatim copy of 0.26.0's header logic: 410 observations of 0-byte .db files and 180 struct.errors in 20 seconds. This one does resolve itself on the next scrape.

2. Permanent, and total. A worker killed inside that window (OOM, SIGKILL during a rolling restart) leaves the empty file behind for good — it is named after a pid that never comes back, so nothing ever cleans it up. Every subsequent scrape then fails, and because the exception aborts the whole merge, all other workers' metrics are lost too. Repro:

import os, tempfile
d = tempfile.mkdtemp()
os.environ['PROMETHEUS_MULTIPROC_DIR'] = d
from prometheus_client import CollectorRegistry, Counter, values
from prometheus_client.multiprocess import MultiProcessCollector
values.ValueClass = values.MultiProcessValue()
reg = CollectorRegistry()
c = MultiProcessCollector(reg)
Counter('good', 'help', registry=None).inc()
# a 0-byte file left behind by a process killed between open('a+b') and truncate()
open(os.path.join(d, 'counter_31337.db'), 'wb').close()
for i in range(3):
    try:
        print(f"  scrape {i+1}: OK, metrics={sorted(m.name for m in c.collect())}")
    except Exception as e:
        print(f"  scrape {i+1}: {type(e).__name__}: {e}")

On 0.26.0 all three scrapes raise, and the unrelated good counter is never exported.

_read_metrics already tolerates the analogous race of a gauge_live* file vanishing between the glob and the read (FileNotFoundError, via mark_process_dead), but there is no equivalent tolerance for a file that exists and has not been initialised — for any metric type.

Suggested fix

Treat an empty read as a file with nothing recorded in it yet, which is exactly what __init__'s own if capacity == 0 branch already does on the write side:

data = infp.read(mmap.PAGESIZE)
if not data:
    return iter(())
used = _unpack_integer(data, 0)[0]

Deliberately narrow, for two reasons:

  • It changes nothing for files that are non-empty. A file claiming more than it holds still raises, and _read_all_values's RuntimeError('Read beyond file size detected, file is corrupted.') is untouched — genuine corruption keeps failing loudly.
  • It is a smaller change than it looks: a 4-to-8-byte all-zero file already reads as empty today, so this extends existing behaviour to the 0-byte case rather than introducing new leniency.

Two alternatives I looked at and would argue against:

  • Catching struct.error in _read_metrics — far too broad, and would swallow real corruption. An os.path.getsize() pre-check instead just adds a syscall per file per scrape and is still TOCTOU.
  • Deleting 0-byte files on read (suggested in #604) — risky: the file may belong to a live worker that holds the fd and is about to truncate it, and unlinking would orphan that worker's metrics for its whole lifetime.

Closing the window at source in __init__ (write to a temp file, then atomic os.rename) is worth considering as a follow-up, but it is a bigger change and, crucially, would not heal empty files already on disk from earlier versions.

Relation to #604

#604 reports this identical error and was closed in 2020, with the suggestion that it was misuse or, in the maintainer's words, "data corruption on disk". I think that conclusion was reached without the permanent-file case being visible; the repro above shows a 0-byte file is enough on its own, with no corruption involved. A 2025 comment on that thread independently pins it on 0-byte .db files, with a directory listing. Happy for this to be folded back into #604 if you'd prefer.

Versions
  • prometheus_client: hit in production on 0.20.0; verified unchanged on 0.26.0 (mmap_dict.py is byte-identical between the two).
  • Python 3.13, Linux (Kubernetes), gunicorn.

I have the fix plus four regression tests ready as a PR against master (DCO signed) — the tests cover the empty file at both the MmapedDict and merge() levels, that other metrics survive a stale empty file, and that a truncated non-empty file still raises. Full suite passes on the tox matrix, flake8 and isort clean. Glad to open it if this looks like the right direction.

LLM use

Please note that I used Claude Opus and Sonnet whilst investigating this.

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

prometheus_client/mmap_dict.py の read_all_values_from_file から始め、次に prometheus_client/multiprocess.py の MultiProcessCollector.merge を調べます。MmapedDict と merge を対象とする回帰テストを実行します。空のファイルは無視され、他のメトリクスは保持され、空でない切り詰められたファイルでは引き続きエラーが発生する必要があります。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
python
領域
observability-sre
issue の種類
バグ
難易度
2/5
見積もり時間
1〜3時間
活発さ
静か
明瞭さ
明確に書かれている
初心者へのやさしさ
58/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。