Skip to content

Skip processLocalRelationshipUpdates when no relationships to write#617

Merged
ZihanLi58 merged 1 commit into
linkedin:masterfrom
ZihanLi58:zihli/skip-empty-relationship-tx
May 6, 2026
Merged

Skip processLocalRelationshipUpdates when no relationships to write#617
ZihanLi58 merged 1 commit into
linkedin:masterfrom
ZihanLi58:zihli/skip-empty-relationship-tx

Conversation

@ZihanLi58
Copy link
Copy Markdown
Contributor

handleRelationshipIngestion is called once per aspect during create/update. For aspects with no
RELATIONSHIP-typed fields, localRelationshipUpdates ends up empty after the existing .filter(entry -> !entry.getValue().isEmpty()) step. The existing code still calls
_localRelationshipWriterDAO.processLocalRelationshipUpdates with that empty list — its body is a no-op
for-loop, but the @Transactional annotation on the method opens + commits a transaction per call
regardless.

On a slow MySQL host this manifests as ~150 ms per aspect of pure transaction overhead (BEGIN + COMMIT
round-trips) for no useful work. With N aspects per create, total wasted time scales linearly.

Evidence

A 5-aspect MlModelInstance/create on prod-lor1 (a LinkedIn fabric) was observed at 867 ms wall-clock, of
which the actual SQL INSERT is 1-2 ms (per dao.benchmark.<entity>.create.aspects_5.latency). A wall-clock
async-profiler flame graph showed the rest of the time spent in:

BaseLocalDAO.create
→ createAspectsWithCallbacks
→ EbeanLocalDAO.createNewAssetWithAspects
→ handleRelationshipIngestion (called once per aspect)
→ EbeanLocalRelationshipWriterDAO.processLocalRelationshipUpdates [@transactional]
→ JdbcTransaction.commit → setAutoCommit → MySQL round-trip → __poll

Linear scaling confirmed by varying aspect count on the slow fabric:

Aspects daoCreate (ms)
1 220
3 542
5 867

Per-aspect cost ≈ 162 ms regardless of whether the aspect actually has any relationships to write. None of
the test payloads contained any populated relationship fields, yet the cost still appeared linearly with N
aspects.

Fix

One-line short-circuit: skip the call to processLocalRelationshipUpdates when the list is empty, avoiding
the no-op @Transactional invocation entirely. Behavior is identical for the non-empty case (the existing
code path is unchanged for aspects that do have relationships to write).

Testing Done

  • Compiles cleanly (./gradlew :dao-impl:ebean-dao:compileJava)
  • Existing relationship-ingestion tests in EbeanLocalDAOTest cover the non-empty path; my change
    preserves identical behavior there. Local MariaDB4j-based test suite couldn't be run in my environment (test
    infra issue unrelated to the change), but the change is observably equivalent for the non-empty case and
    only avoids a no-op call in the empty case.
  • Plan to verify end-to-end on prod-lor1 after pickup, expecting create latency to drop from ~600 ms to ~50
    ms for the same payload (matching prod-ltx1 baseline).

Checklist

handleRelationshipIngestion is called once per aspect during create/update.
For aspects with no RELATIONSHIP-typed fields, localRelationshipUpdates ends
up empty after the .filter(entry -> !entry.getValue().isEmpty()) step. The
existing code still calls _localRelationshipWriterDAO.processLocalRelationshipUpdates
with that empty list — its body is a no-op for-loop, but the @transactional
annotation on the method opens + commits a transaction per call regardless.

On a slow MySQL host this manifests as ~150ms per aspect of pure transaction
overhead (BEGIN + COMMIT round-trips) for no useful work. With N aspects per
create, total wasted time scales linearly: e.g. 5-aspect MlModelInstance
create observed at 867ms wall on prod-lor1, of which the inserted SQL is
1-2ms (per dao.benchmark.<entity>.create.aspects_5.latency).

Fix: short-circuit when localRelationshipUpdates is empty, avoiding the
no-op @transactional invocation entirely.
@ZihanLi58 ZihanLi58 closed this May 6, 2026
@ZihanLi58 ZihanLi58 reopened this May 6, 2026
@ZihanLi58 ZihanLi58 merged commit 32b85d6 into linkedin:master May 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants