> Anatomy of an N+1: How One Redundant findById Turned 50 Updates into 1000 Queries

Our APM tooling flagged a suspicious pattern in one of our background jobs: the same single-row SELECT fired 50+ times per run, in pairs. A cache-warming job that should have cost a handful of queries was generating around a thousand.

This is a walkthrough of that investigation — because this N+1 didn’t come from lazy loading, which is where everyone looks first. It came from two innocent-looking lines of code and a loop that multiplied them by ten.

Names of classes, methods, and domain objects below are changed — the shape of the problem is exactly what shipped.

The setup

The job in question warms a per-tenant cache of content items, across 10 timezones:

RefreshContentCacheJob.execute()
  └─ for each tenant:
       warmTimezoneCaches(tenantId, TIMEZONES)       // 10 timezones
         └─ listForTenant(tenantId)                   ← 1 bulk JOIN query (good)
         └─ for each timezone:
              applyItemFilters(items, timezone)
                └─ isAssetFresh(item)                 ← called for EVERY item × EVERY timezone
                     └─ markItemAsStale(item)         ← when the asset's freshness window expired
                          └─ repository.findById(id)  ← SELECT #1
                          └─ repository.save(item)    ← em.merge() → SELECT #2 + UPDATE

The bulk load at the top was fine — one JOIN query per tenant, using an entity graph. The damage happened further down, and it took two separate bugs working together.

Issue 1: the redundant findById

Here’s markItemAsStale, the method at the bottom of that call stack:

// BAD — entity `item` is already in memory, no need to re-fetch
@Transactional
public void markItemAsStale(ContentItem item) {
    Optional<ContentItem> found = itemRepository.findById(item.getId()); // SELECT (unnecessary!)
    if (found.isPresent()) {
        found.get().setStale(true);
        itemRepository.save(found.get()); // em.merge() → possible SELECT + UPDATE
    }
}

The method receives a fully loaded entity and immediately re-fetches it by ID. The findById adds zero correctness value — item was returned by a repository query moments earlier. The Optional.isPresent() guard protects against an ID that cannot be missing.

It gets worse. The entity had FetchType.EAGER on all of its collections:

@ElementCollection(fetch = FetchType.EAGER) Set<Integer> displaySlots;
@ElementCollection(fetch = FetchType.EAGER) Set<String> skuCodes;
@ManyToMany(fetch = FetchType.EAGER) Set<Tag> tags;
@ManyToMany(fetch = FetchType.EAGER) Set<Tag> excludedTags;

So that “one little findById” was not a cheap primary-key lookup. It was a ten-way LEFT JOIN monster — tags, exclusions, SKUs, attachments — all to flip a single boolean. And em.merge() on the freshly fetched entity fired a second SELECT before the UPDATE. That’s the “pattern of 2” APM detected.

Issue 2: the multiplier hiding in the loop

One redundant fetch per stale item would be bad. But the filter that triggers it ran inside the timezone loop:

// applyItemFilters — called once per timezone
items.stream().filter(item ->
    isNotExpired(item, timezone)
    && isValidFor(item, timezone)
    && isAssetFresh(item)   // ← timezone-INDEPENDENT, but called per timezone!
    && item.isActive())

isAssetFresh checks lastModified + freshnessWindow against wall-clock time. It has no timezone dependency whatsoever. Yet it sat between two predicates that do, so it ran once per item per timezone — and so did its database side effect.

The math: N stale items × 10 timezones × 2 queries each. Fifty stale items became 1000 queries where 50 updates would do.

There’s a second lesson buried here: isAssetFresh looks like a pure predicate but writes to the database. A side effect hiding inside a stream filter is exactly the kind of thing that gets multiplied when someone wraps the call in another loop.

The fixes

Fix 1: operate on the entity you already have

@Transactional
public void markItemAsStale(ContentItem item) {
    item.setStale(true);
    itemRepository.save(item);
}

If item happens to be detached, save → em.merge() re-attaches it correctly anyway. The manual pre-fetch bought nothing.

Fix 2: hoist the timezone-independent check out of the loop

public void warmTimezoneCaches(String tenantId, List<String> timezones) {
    List<ContentItem> items = contentService.listForTenant(tenantId);

    // Asset freshness is timezone-independent — evaluate once, mark stale once.
    List<ContentItem> freshItems = items.stream()
            .filter(this::isAssetFresh)
            .collect(Collectors.toList());

    timezones.forEach(timezone -> {
        List<ContentItem> filteredItems = freshItems.stream()
                .filter(item -> isNotExpired(item, timezone)
                        && isValidFor(item, timezone)
                        && item.isActive())
                .collect(Collectors.toList());
        contentCache.set(getCacheKey(tenantId, timezone), /* ... */);
    });
}

markItemAsStale now fires at most once per stale item per run instead of ten times. O(N) instead of O(N×T).

Fix 3 (the follow-up): one bulk UPDATE instead of N

Even after fixes 1 and 2, N stale items still cost N individual UPDATE round-trips. A @Modifying query collapses them into one:

@Modifying
@Query("UPDATE ContentItem c SET c.stale = true WHERE c.id IN :ids")
int setStaleForIds(@Param("ids") List<Long> ids);

Collect the stale IDs during filtering, call this once per tenant. One query whether 5 items are stale or 500.

One caveat worth knowing: bulk JPQL updates bypass Hibernate’s first-level cache — already loaded entities won’t reflect stale = true unless you also update them in memory (or never re-read them, as in this call path). And if the ID list can grow into the hundreds, chunk it — some databases limit IN clause size.

The impact

Scenario	Before	After
50 stale items, 10 timezones	50 × 10 × 2 = 1000 queries	50 updates (fix 1+2), 1 update with fix 3
0 stale items	0 extra	0 extra
Per-tenant baseline	1 bulk SELECT	1 bulk SELECT

Takeaways

• N+1 isn’t always lazy loading. This one was eager loading plus a redundant re-fetch. A findById on an entity with EAGER collections is a full JOIN query, not a cheap lookup.
• Never re-fetch what you already hold. If a method receives a managed entity, mutate it and save it. The defensive findById + isPresent() dance costs round-trips and proves nothing.
• Audit your loops for invariant work. A per-item cost is bad; a per-item cost inside a loop that doesn’t need it is a multiplier. Ask of every call in a loop: does this actually depend on the loop variable?
• Keep side effects out of predicates. A filter that writes to the database will eventually be called somewhere its author didn’t anticipate.
• APM earns its keep. This pattern survived code review because each piece looked reasonable in isolation. The “2 queries × 50 occurrences” grouping in the trace view is what made the shape obvious.

Links

• Video on YouTube
• Discussion on LinkedIn