feat(apply): eliminate per-m2m serialize queries in buffered change logging#177
Merged
Conversation
mfiedorowicz
requested review from
MicahParks,
jajeffries,
leoparente,
ltucker and
marc-barry
as code owners
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
mfiedorowicz
changed the title
mfiedorowicz
changed the title
…ingle bulk_create NetBox's handle_changed_object receiver writes one core_objectchange row per saved model (and for m2m_changed events does a SELECT + UPDATE to merge into any prior entry for the same instance in the request). A 50-entity bulk-plan-apply with a couple of m2m fields per entity is ~150-300 DB round-trips for change logging alone; the existing apply_bypass_change_logging shortcut sheds that cost by dropping the audit trail entirely. This change adds a third option: collect ObjectChange instances in a request-scoped contextvar buffer during apply, with in-memory m2m dedup (replacing the per-row SELECT), and flush them as one ObjectChange.objects.bulk_create on successful exit. post_save is manually re-emitted for each flushed row so plugins that consume the signal (netbox-branching's record_change_diff, eventsink) still fire. The events_queue receives appends exactly as upstream, so webhook emission is unchanged. Gated by a new plugin setting apply_buffer_change_logging (default False). If apply_bypass_change_logging is also active the bypass wins (predictable combined semantics). If the apply transaction rolls back the flush is skipped and any in-memory ObjectChange rows are dropped. Tests cover: setting-off pass-through, setting-on bulk_create flush, row equivalence vs unbuffered, rollback drops buffered rows, post_save re-emit fires per flushed row, bypass-wins precedence.
…t cascades The previous flush reset the buffer contextvar BEFORE calling bulk_create and re-emitting post_save. Receivers connected to post_save sender=ObjectChange (notably NetBox's update_denormalized_fields) save related models during the re-emit; those cascading saves fired _buffered_handler with buffer=None and fell through to upstream handle_changed_object, producing single-row INSERTs into core_objectchange after every bulk_create. Observed in production as ~90% of ObjectChange writes still being single-row despite the buffer setting being on. Keep the contextvar active across the flush and drain to fixpoint: clear the buffer dict in place, bulk_create + re-emit, then loop while the dict has new entries (cascading saves landing back in the same buffer). Bounded to 5 iterations as a safety; NetBox denormalisation in practice cascades once. Add a regression test that connects a fake receiver simulating the denormalisation cascade and asserts the cascading save lands as an ObjectChange row via bulk_create rather than via the upstream single-row INSERT path.
…nchronous bulk_create
The synchronous flush replaced individual INSERTs with a single
bulk_create, but profiling showed that wasn't where the cost lived.
The cost of change-logging is dominated by the per-save Python work
in handle_changed_object and to_objectchange (FK lookups, JSON
serialisation of pre/post state, fan-out across all post_save
receivers, Postgres FK locks acquired during each ObjectChange
INSERT). Batching INSERTs did not measurably move the apply
throughput needle.
Move the entire ObjectChange creation off the apply request critical
path:
- Buffer still collects ObjectChange instances in memory during apply
(cheap, in-process, in-memory).
- At successful end of apply, the buffer is serialised to a plain
dict payload and an RQ job is enqueued via transaction.on_commit
using django_rq.get_queue('default').enqueue with rq.Retry for
exponential backoff. Apply transaction commits without paying any
ObjectChange write cost.
- New async_change_logging.write_object_changes_async runs in the
worker: rebuilds ObjectChange instances from the payload,
bulk_creates them, and re-emits post_save for each row so any
receiver connected to post_save(sender=ObjectChange) still fires.
active_branch contextvar is re-established from the payload when
netbox-branching is installed.
- Rollback semantics preserved: transaction.on_commit only fires on
successful commit, so a failed apply discards the payload.
- Worker failures retry 3x with 5s/30s/120s backoff then land on the
RQ failed queue.
django_rq.enqueue chosen over JobRunner because at expected apply
volumes (~50k applies/day at 30 e/s) JobRunner's per-execution
core_job row would bloat the table without operational value;
django_rq matches the pattern NetBox already uses for webhook
delivery in extras/events.py.
Trade-off: audit log becomes eventually consistent (typical lag <1s
with a healthy queue). Reads of core_objectchange immediately after
apply may miss the just-applied changes. Gated behind
apply_buffer_change_logging (default False) so this is opt-in.
Tests cover: setting-off pass-through, setting-on enqueue with
payload shape verification, rollback does not enqueue, bypass takes
precedence, and direct unit tests of the worker entry point (build
ObjectChange from payload, bulk_create, signal re-emit, empty
payload no-op).
Previously every entity in a bulk-plan-apply / bulk-apply request enqueued its own RQ job carrying that entity's buffered ObjectChange rows. With ~1 row per entity that meant the worker ran one bulk_create([1]) per entity - same single-row INSERT shape as a direct Model.save(), and one RQ job per entity (50x per typical request). Add a new request-scoped context manager `request_change_logging_batch` that wraps the entity loop in the bulk endpoints. Per-entity `buffered_change_logging` now appends its payload to a shared list via `transaction.on_commit` instead of enqueueing directly. On exit, the wrapper consolidates all appended payloads into one job and enqueues it. The worker now runs a single `bulk_create([N])` for the whole request, using the UNNEST shape and a single Postgres round-trip for all ObjectChange writes. Per-entity rollback semantics preserved: each entity's append is registered via its own atomic block's `on_commit`. A failed entity's atomic rolls back and Django discards its callback, so the batch contains only rows from entities that genuinely committed. Wrapping registration of the consolidate-and-enqueue in `finally` ensures partial batches still land when a later entity raises an unhandled exception - prevents committed model writes from going un-audited. The fallback per-entity enqueue path remains in place for callers that don't wrap their loop (single-changeset endpoint). Tests: 3-entity request enqueues once with a 3-row payload; bad entity in the middle of a batch is excluded from the consolidated payload while the good entity's row is preserved. Mirrored across v4.4.x / v4.5.x / v4.6.x.
…ogging
The buffered change-logging path served as a buffer in front of the
synchronous ObjectChange write, but the throughput cost was never the
write. NetBox's `to_objectchange` runs Django's
`serializers.serialize('json', [obj])`, which issues one SELECT per
many-to-many relation on the model (its `handle_m2m_field` walks each
m2m manager). For a model with three m2m relations that is three DB
round-trips on every save, dominating the apply request critical path.
Address the actual cost in three pieces:
- Fast serialisation: while a diode apply buffer is active,
`ChangeLoggingMixin.serialize_object` is routed through
`_fast_serialize_object`, which restricts Django's serializer to the
model's local non-m2m fields via the `fields=` allowlist. Django then
skips `handle_m2m_field` entirely, so no per-relation SELECT is
issued, while its own field rendering is preserved unchanged. Gated on
the buffer contextvar, so all non-apply change logging is byte-for-byte
identical to upstream.
- Bulk m2m enrichment: the omitted m2m fields are re-added at flush. Rows
are grouped by model and one through-table query per relation resolves
the relation for every buffered object at once, turning
O(saves x relations) round-trips into O(models x relations) per
request. Membership matches what the upstream serializer records.
- Inline flush: the buffer is flushed as a single `bulk_create` at
`on_commit`, with `post_save` re-emitted so receivers connected to
`post_save(sender=ObjectChange)` still fire. The audit trail stays
synchronous and immediately consistent.
This removes the separate worker write path and its eventual-consistency
trade-off entirely.
Tests rewritten to cover fast-serialise parity (output equals the
upstream serializer minus m2m), the buffer-active gate, bulk enrichment
(one query per relation), and the end-to-end apply behaviour (setting
off/on, rollback, bypass precedence, request batching, failed-entity
exclusion). Verified against NetBox v4.5.5 (354 tests) and v4.6.0 (359
tests).
…save `_fast_serialize_object` resolved tags with `obj.tags.all()` on every save, which under bulk apply is one query per saved object (and twice for objects that also fire m2m_changed). The per-relation m2m queries were already moved to a single batched flush; tags were the remaining per-save round-trip in serialisation. Leave an empty `tags` placeholder during serialisation - added only for taggable models, which also marks the row as needing tag enrichment - and fill it in `_enrich_tags` at flush: rows are grouped by content type and one query over `extras_taggeditem` resolves every buffered object's tag names at once, written back as a sorted list. Output matches the upstream serializer (non-taggable rows keep no `tags` key). Reads run post-commit, so they observe the final tag assignments. Tests cover per-object tag resolution, single-query batching, the empty and no-placeholder cases, and full serialiser parity (fast serialize + m2m + tag enrichment equals the upstream output). Verified on NetBox v4.5.5 (358 tests).
mfiedorowicz
changed the title
mfiedorowicz
force-pushed
the
prs/buffered-change-logging
branch
from
373a482 to
d93bba8
Compare
NetBox captures the pre-change state of an object by calling
`instance.snapshot()` in its view and DRF viewset layers
(`get_object_with_snapshot`, the bulk update/destroy mixins). The apply
path applies through a plain APIView and a direct `serializer.save()`,
so it bypassed those entirely and recorded no `prechange_data` for
updates - the audit log showed only post-change state, unlike every
update made through NetBox's own UI or REST API. Pre-existing gap,
independent of the buffered change-logging work.
Add `snapshot_for_apply` and call it after fetching the instance and
before saving, in both update paths (update by id, and the
find-existing-then-update path used for pre-save-match and auto-created
component types). The create-then-update-in-one-changeset path is left
alone: the instance is freshly created, so there is no prior state to
snapshot.
The prechange must be format-consistent with the postchange or the
changelog diff reports spurious changes:
- Buffer inactive: defer to NetBox's `snapshot()` (full serialiser),
which matches the unbuffered postchange exactly.
- Buffer active: build the prechange the same way the buffered
postchange is built - scalar fields via the fast serialiser, m2m and
tags resolved now and sorted to match the flush-time enrichment. They
must be read at snapshot time because the before-state is gone once
the update commits, so enrichment cannot recover them. This is one
read per m2m relation plus one for tags, per updated object - paid
only on updates, only for the before-state.
Tests assert the buffer-active snapshot records sorted m2m + tags, that
prechange equals postchange for an unmodified object (no spurious diff),
and that the buffer-inactive path delegates to NetBox's snapshot.
Verified on NetBox v4.5.5 (361 tests).
jajeffries
approved these changes
Jun 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Change logging on the apply path is expensive. NetBox's
to_objectchangeruns Django'sserializers.serialize('json', [obj]), whosehandle_m2m_fieldissues one SELECT per many-to-many relation on the model. For a model likedcim.Interface(three m2m relations) that is three DB round-trips on every save. Profiling the apply path attributed roughly 40% of per-request cost to this single call, almost entirely to those m2m round-trips (measured ~9-12ms per round-trip; the JSON round-trip itself and tag resolution were minor by comparison).An earlier iteration of this PR moved the ObjectChange write to background RQ workers. That did not move throughput, because the cost was never the write - it was building the row. This revision targets the actual cost.
Change
Three pieces, all gated by
apply_buffer_change_logging(default off):Fast serialisation. While a diode apply buffer is active,
ChangeLoggingMixin.serialize_objectis routed through_fast_serialize_object, which restricts Django's serializer to the model's local non-m2m fields via thefields=allowlist. Django skipshandle_m2m_fieldentirely, so no per-relation SELECT is issued, while its own field rendering is preserved (FKs and scalars unchanged). The route is gated on the buffer contextvar, so all non-apply change logging (UI, REST API, other receivers) is byte-for-byte identical to upstream.Bulk m2m enrichment. The omitted m2m fields are re-added at flush. Buffered rows are grouped by model and one through-table query per relation resolves the relation for every object at once, turning O(saves x relations) round-trips into O(models x relations) per request. Membership matches what the upstream serializer records (recorded as sorted PK lists; identical membership, order may differ from queryset order).
Inline flush. The buffer is flushed as a single
bulk_createaton_commit, withpost_savere-emitted so receivers connected topost_save(sender=ObjectChange)still fire. The audit trail stays synchronous and immediately consistent.The separate worker write path (
async_change_logging.py) and its eventual-consistency trade-off are removed.Behaviour and safety
_fast_serialize_object(obj)equals the upstream serializer output minus m2m fields; after enrichment, m2m membership matches upstream. Covered by tests on two core versions.serialize_objectroute is a process-wide class patch but inert unless the buffer contextvar is set, which only happens inside a diode apply. The contextvar isolates per request/thread and is reset infinally. Non-apply paths pay only a contextvar check.apply_bypass_change_loggingstill takes precedence: with both enabled, no rows are produced.on_commit); a failed entity contributes nothing to the request batch.Testing
Rewritten test suite covers fast-serialise parity, the buffer-active gate, bulk enrichment (asserts one query per relation), and end-to-end apply behaviour (setting off/on, rollback, bypass precedence, request-level batching, failed-entity exclusion).
Pending
Throughput gain to be confirmed on a load-test deployment (expected to lift the per-save serialise cost from tens of milliseconds to sub-millisecond plus tag resolution).