A destruct() Flush Turns Thousands of Redundant Saves Into Hundreds

Submitted by charles on

The Problem

An event subscriber that does real work on every event it sees looks perfectly correct, right up until the events arrive in a batch. This one saved a shared record once per event. At one event at a time that was fine. The moment a sync fired a hundred of them at once, it quietly became thousands of redundant saves. Here's how that happened, and the small change that fixed it without slowing the events down at all: accumulate the work as it comes in, and do it once at shutdown.

The Fix

The fix wasn't to make each save cheaper (although we did that too). It was to stop saving on every event. The handler stopped touching storage at all. It just records which ids changed, and returns:

public function onRecordDeactivated(RecordChangedEvent $event): void {
  $id = $event->getRecord()->id();
  $this->pending[$id] = $id; // repeated ids in the same run dedupe for free
}

The real work, removing each pending id from the groups that actually contain it, runs exactly once, right at the end of the request or command. Most runtimes give you a hook for this. In PHP I implemented the "needs destruction" interface and tagged the service, so the container calls the method on shutdown:

public function destruct(): void {
  if (!$this->pending) {
    return;
  }
  $ids = array_values($this->pending);
  $this->pending = [];
  // Re-check status now, not when the event fired - a record
  // deactivated and reactivated within the same run should be left alone.
  $still_inactive = $this->lookUpCurrentlyInactive($ids);
  foreach ($this->findGroupsContaining($still_inactive) as $group) {
    $group->remove(array_intersect($still_inactive, $group->memberIds()));
    $group->save(); // once per group, no matter how many ids it lost
  }
}

Now the cost tracks the number of groups that actually lost a member, not people times groups. In the batch that used to fire several thousand saves, only a couple of hundred groups had really changed. Save calls dropped by something like 30x, and the run finished in a fraction of the time.

Two things kept this correct rather than just fast. First, re-check state at flush time, not when the event fired. What you accumulate is a list of candidates, not a list of facts. Someone deactivated early in the batch might be reactivated before the flush runs, so I look up current status inside destruct() and skip anyone who is no longer inactive. The second one is easier to miss: iterating every affected group and calling remove() blindly re-serializes groups that never held the person in the first place, which drags the cost straight back up. Intersect the pending ids against each group's real membership first, and you only save the groups that genuinely changed.

The Takeaway

This is just accumulate-then-flush, and it turns up well beyond one event subscriber. Any time a handler's real job is "update a shared thing that a lot of events point at," doing that update inline becomes a save per event, and your most popular records take a save for every event that so much as touches them. Move the update to an end-of-life hook instead, whether that's a destructor, an `atexit` handler, or a `finally` wrapped around your dispatch loop, and save-per-event becomes save-per-container. Nothing about how or when the events fire has to change. 

Two things are worth keeping in mind before you reach for it:

The pending state lives in memory until the flush runs, so a hard kill mid-run loses whatever hasn't flushed yet. If the event that would re-trigger the work isn't going to fire again for something you already swallowed, that's a real gap and not just a delay. Decide up front whether you need a reconciliation job as a backstop, or whether the event is rare and cheap enough that you can live with the risk. In my case I added a drush command and had a script run it hourly. 

And collapsing N saves into one doesn't make that one save free. If updating a single container is expensive on its own, say it has to decode and re-encode a large document just to drop one entry, then funnelling hundreds of those into a single deferred pass can turn a clean win into a new and very visible bottleneck once the volume climbs. The redundant fan-out and the cost of a single update are two separate problems, and fixing the first doesn't touch the second. So when a lot of small events all want to touch the same few records, collect the intent and flush it once. Just re-check before you act, and don't assume the flush itself is cheap.