perf: index metricarchive.metricid used by the metrics archive job - #2918
Open
constantine2nd wants to merge 1 commit into
Open
constantine2nd wants to merge 1 commit into
constantine2nd wants to merge 1 commit into
Conversation
MetricsArchiveScheduler looks each archived row up by metricId twice: once in saveMetricsArchive (dedup) and once before deleting the source row (verify). MetricArchive declared no index on metricId, so both lookups were full scans of metricarchive and the cost of archiving a single row grew linearly with the size of the archive. Adding Index(metricId) to MetricArchive.dbIndexes makes both lookups index scans. Schemifier creates the missing index on existing deployments at boot.
|
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
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
MetricsArchiveScheduler.conditionalDeleteMetricsRowmoves oldmetricrows tometricarchiveone row at a time, and for every row it looks the archive row up bymetricIdtwice:MappedMetrics.saveMetricsArchive-MetricArchive.find(By(MetricArchive.metricId, primaryKey))(dedup before insert)conditionalDeleteMetricsRow-MetricArchive.find(By(MetricArchive.metricId, i.getMetricId()))(verify before deleting the source row)MetricArchive.dbIndexeshas no index onmetricId, so on PostgreSQL both lookups are sequential scans of the wholemetricarchivetable. The cost of archiving one row therefore grows linearly with the size of the archive.Evidence
metricarchivewith 1,542 sequential scans for a run that moved 766 rows: two full scans per archived row. That run took 19.5 s (about 25 ms per row).Index(metricId)With the default
retain_metrics_move_limitof 10,000 rows per run, a 15M-row archive would need several hours per run instead of fitting the ~10 minute scheduler interval; theJobSchedulerlock then skips the following runs and the livemetrictable keeps growing.Change
Add
Index(metricId)toMetricArchive.dbIndexes(one line, with a comment explaining why).Deployment note
Schemifier creates missing indexes at boot, so existing deployments get
metricarchive_metricidautomatically on the next start. It is a plainCREATE INDEX, which blocks writes tometricarchivewhile it builds. Only the archive job writes to that table, and the build is fast (seconds per few million rows on commodity hardware), so no separate migration is added.Testing
Not compiled locally (the build requires JDK 25, which is not installed on this machine); relying on CI. The change follows the existing
dbIndexespattern;MappedLongfields are already used in index declarations elsewhere (for exampleUniqueIndex(mCustomerId, ...)inMappedTaxResidence).