Page MenuHomePhabricator

Make pickle writes atomic
Open, Needs TriagePublic

Description

pickle_dump opens the target in write-binary mode, which truncates it immediately, then streams gzip into it. A worker that dies mid-write (soft-timeout SIGTERM, hard-limit SIGKILL, OOM) leaves a truncated file where the old pickle was, and the next read fails. This happens in production: a long-queue worker dies on a zlib decompress error, which is a half-written gzip pickle. The corrupt-pickle path then discards it and reparses from scratch, and for a large article that reparse times out and ends up on the stuck long queue (T422230), so one interrupted write can make an article unavailable indefinitely.

Writing to a temp file and renaming it into place is the fix, but a first attempt broke the file locking, the permissions, and temp-file cleanup, so let's not be naive.

Acceptance criteria

  • An interrupted write leaves the previous pickle intact, never a truncated or zero-byte file at the real path
  • Concurrent writers stay serialized across the rename (lock the subdirectory or another stable target):
    • locking the file being replaced fails because os.replace swaps the inode out from under the lock
    • the per-page memcached lock is not sufficient on its own
  • Rewritten pickles keep 0644 (or the umask-derived mode)
    • the backup cron and the cutover rsync can't read the 0600 files that tempfile.mkstemp creates
  • A failed temp-file creation cannot leave a zero-byte file that later reads treat as a valid pickle
  • Orphaned temp files from a SIGKILL between create and rename are reaped rather than accumulating on the pickle volume (extend cleanup_legacy_pickles or sweep on worker start)
  • A test trips the interrupted-write failure without patching the shared cPickle module process-wide