docs(dao): add batch-upsert long-tail latency analysis - #630
Open
jphui wants to merge 6 commits into
Open
Conversation
Document production evidence that BatchUpsert=true regresses or is flat at CountBucket=1 (notably dataset/datasetinstance, MGA's dominant write volume) while winning decisively at CountBucket>=2, and trace the root cause through EbeanLocalAccess.batchUpsert()/prepareMultiColumnInsert() vs. the existing single-aspect add() path: a generic batch-get read path, duplicated classNames/AspectKey derivation, and a String.format scan over the fully-assembled SQL string, none of which the single-aspect path pays. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Trim production-evidence prose, condense code-level comparison to concise per-hypothesis summaries, correct the old-value read finding (single-aspect and batch paths share the same batchGetUnion / per-aspect-column read), and add a 'why not do anything now' section concluding the net workload win outweighs the ~100-200us dataset bucket-1 regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a TODO in EbeanLocalAccess.batchUpsert() pointing to docs/batch-aspect-upsert-long-tail-latency.md for the N=1 fixed-overhead follow-up, and apply spotless markdown formatting to the analysis doc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #630 +/- ##
=========================================
Coverage 66.93% 66.93%
Complexity 1889 1889
=========================================
Files 148 148
Lines 7280 7280
Branches 883 883
=========================================
Hits 4873 4873
Misses 2021 2021
Partials 386 386 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Reduce the production-evidence table to the dataset bucket-1 row (MGA's highest-volume write) and correct it to the actual measured regression: per-aspect 0.308 ms to batch-upsert 0.441 ms (+43.2%). Clarify that a positive Avg % delta denotes a regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the raw production-evidence section and internal service names, generalize the summary and 'why not do anything now' rationale to avoid specific numbers/counts, and drop the concrete latency figure and entity names from the batchUpsert TODO. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
Adds
docs/batch-aspect-upsert-long-tail-latency.md, a code-level root-cause analysis of thebatch-upsert write path's long-tail latency behavior.
Production latency data from the MGA batch-upsert ramp shows
BatchUpsert=truewins decisively athigher aspect counts (
CountBucket >= 2) but is flat or slightly slower than the existingper-aspect
dao.add()path atCountBucket = 1— most visibly ondatasetanddatasetinstance,which dominate MGA's write volume.
The doc captures the production evidence and traces both code paths
(
BaseLocalDAO.addManyBatchInternal/EbeanLocalAccess.batchUpsertvs.aspectUpdateHelper/addWithOptimisticLocking) to attribute the single-aspect (N=1) overheadto constant-factor Java work — duplicated
AspectKey/classNamesderivation, extracollection/stream allocation in the old-value read wrapping, and a full-string
String.formatscan over the assembled multi-column upsert SQL — rather than reflection or extra DB round-trips.
It closes with suggested follow-ups in priority order (N=1 fast-path, de-duplicated derivation,
allocation trims) and a note on validating with allocation profiling / a JMH microbenchmark.
Docs-only change — no production code is modified.
Testing Done
N/A — documentation-only change. Analysis was derived from reading the existing DAO code paths and
72h production latency data; no code behavior changes.
Checklist
🤖 Generated with GitHub Copilot CLI