Fix deadline callback data encoding in migration 0094 (scheduler CrashLoopBackOff)

[seanmuth]

Closes #69980.

What's the problem

Migration 0094_3_2_0_replace_deadline_inline_callback_with_fkey moves inline deadline callbacks into the callback table, whose data column is ExtendedJSON — read back at runtime via BaseSerialization.deserialize, which requires every nested dict to be wrapped as {"__type": "dict", "__var": {...}}.

Both upgrade paths built callback.data without recursively extended-serializing the callback kwargs:

• Postgres (_upgrade_postgresql): json_build_object wrapped only the top level, embedding the old callback's kwargs jsonb raw.
• MySQL/SQLite (_upgrade_mysql_sqlite): wrote the callback dict via a plain JSON column with no extended wrapping at all.

So any callback whose kwargs is a dict — nested, flat, or even empty ({}) — is stored unreadable, since BaseSerialization.deserialize requires kwargs itself to be wrapped, not just anything nested inside it. When the scheduler loads it in the deadline-processing loop, deserialize recurses into the outer dict node, hits the bare kwargs object, runs encoded_var[Encoding.VAR], and raises KeyError(<Encoding.VAR: '__var'>) — crashing the scheduler on every loop (CrashLoopBackOff), before it heartbeats, with no OOM and no other logged exception.

What's in this PR

• Both upgrade paths now extended-serialize the callback data, matching BaseSerialization.serialize:
  • Postgres: a recursive, session-local pg_temp.encode_extended(jsonb) function used inside the existing batch CTE (keeps the one-round-trip design).
  • MySQL/SQLite: a _serialize_extended() helper applied to the callback dict.
• Both downgrade paths decode kwargs back to raw (pg_temp.decode_extended / _deserialize_extended), so an upgrade→downgrade round-trip is preserved. Both decoders are lenient, so they also correctly downgrade data written by the pre-fix version of this migration.
• Fixed a NULL-corruption edge case in the Postgres encode/decode SQL functions: jsonb_agg over an empty array returns SQL NULL, not '[]'::jsonb, so an empty-list callback value would silently become null instead of round-tripping as []. Both functions now COALESCE to '[]'::jsonb.
• Tests (test_0094_deadline_callback_migration.py): existing assertions now verify callback.data round-trips through the real BaseSerialization; added a nested-kwargs/tags regression test, an encode/decode helper round-trip test, and a Postgres-backend test (@pytest.mark.backend("postgres")) covering the empty-list fix directly against the real SQL functions.

Already-affected deployments

This hotfix protects deployments that have not yet run 0094. Deployments already upgraded across 0094 with nested-dict callback kwargs have latent corrupt callback.data rows (a scheduler crash that fires when such a deadline becomes due); those rows must be repaired out of band — e.g. re-encode the affected rows:

-- find affected rows
SELECT count(*) FROM callback c
WHERE jsonb_path_exists(c.data::jsonb, '$.**.__var.* ? (@.type()=="object" && !exists(@.__type))');

---

🤖 Opened with Claude Code (model: Claude Opus 4.8, claude-opus-4-8[1m]).

[steveahnahn]

Verified both paths, _serialize_extended matches canonical BaseSerialization.serialize and round-trips through the real deserializer across nested/empty/list/primitive shapes, and on a live Postgres the pg_temp.encode_extended/decode_extended functions produce the same result and invert cleanly.

nit: the plpgsql path has no test. test_0094_deadline_callback_migration.py builds its engine with _make_engine() (in-memory SQLite), so even the Postgres CI job never executes encode_extended/decode_extended

[aaron-y-chen]

Thanks for the PR!

[aaron-y-chen]

I wonder if node = [], whether the entire return value would be None. I guess that's not what you want, right? Should we handle this case here?

[seanmuth]

Good catch — confirmed this is a real bug: jsonb_agg over zero rows returns SQL NULL, not '[]'::jsonb, so node = [] would silently become null instead of round-tripping as []. Fixed with COALESCE(..., '[]'::jsonb), verified against a live Postgres instance, and added a regression test (TestMigration0094PostgresEncodeDecodeHelpers) that exercises the real SQL functions directly — pushed.

---

Drafted-by: Claude Sonnet 5; reviewed by @seanmuth before posting

[aaron-y-chen]

Same as above.

[seanmuth]

Same fix applied here too — pushed.

---

Drafted-by: Claude Sonnet 5; reviewed by @seanmuth before posting

[kaxil]

Scoping this to nested dicts undersells it. BaseSerialization.deserialize fetches encoded_var[Encoding.VAR] on any non-primitive, non-list value, so a flat kwargs={"k": "v"} and even kwargs={} raise KeyError('__var') exactly like the nested case, and on MySQL/SQLite the pre-fix path wrote no top-level envelope at all so the outer dict raises immediately. Every row 0094 migrated is unreadable, on both backends, not just the ones with nested tags.

Your own diff shows it: test_null_callback_does_not_crash covers a row whose kwargs is {}, and swapping its json.loads for BaseSerialization.deserialize only passes because of this fix. Worth rewording to something like "every deadline callback row migrated by 0094", otherwise an operator whose tags are flat strings reads this release note and concludes they are in the clear.

[seanmuth]

You're right, this undersold it — reworded the newsfragment and PR description: any dict-shaped kwargs (flat, or even empty {}) hits the same KeyError, not just ones with further nested structure, since deserialize requires kwargs itself to be wrapped regardless of what's inside it.

---

Drafted-by: Claude Sonnet 5; reviewed by @seanmuth before posting

[kaxil]

0094 is in the released 3.2.0 tag and main is already 3.4.0, so patching it in place only helps deployments upgrading straight from <3.2. Everyone who went through 3.2.x or 3.3.x has already run the broken version, has 100% of their callback rows unreadable, and will never re-run 0094 to pick this up.

Is a repair revision planned as a follow-up? A new migration could reuse these two helpers to re-encode existing rows, and it would need to handle both corrupt shapes: Postgres rows have the outer __var envelope with raw kwargs underneath, MySQL/SQLite rows have no envelope at all. That second shape is why "repair out of band" is not actionable as written -- the jsonb_path_exists query in the description matches on $.**.__var.*, which finds nothing in a MySQL/SQLite row and so reports it clean.

[seanmuth]

Agreed this only helps deployments that haven't run 0094 yet — that's what the PR body's "Already-affected deployments" section covers: already-upgraded deployments need the out-of-band repair (SQL provided there), since patching an already-applied migration can't retroactively fix rows it already wrote. This split (hotfix-in-place + out-of-band repair, no new repair migration) was what Daniel Standish suggested on the original issue. Let me know if you think that approach needs revisiting.

---

Drafted-by: Claude Sonnet 5; reviewed by @seanmuth before posting

[ferruzzi]

@seanmuth are you still working on this one?

[seanmuth]

Yep, still working on it — just let it sit too long. Pushed fixes today for Aaron's and Kaxil's review (a real empty-list encoding bug Aaron caught, plus scope/wording corrections from Kaxil) and rebased onto main. Should be ready for another look.

---

Drafted-by: Claude Sonnet 5; reviewed by @seanmuth before posting

[github-actions]

uv.lock on main just moved via commit cdcbf12 and this PR currently conflicts.

Quickest fix:

git fetch upstream main && git rebase upstream/main
rm uv.lock && uv lock
git add uv.lock && git rebase --continue
git push --force-with-lease

Automated nudge — ignore if you're not ready to rebase. This comment is updated in place on future uv.lock bumps.