perf: consolidate the Frames indexes, and drop unused and duplicate indexes elsewhere - #5118
perf: consolidate the Frames indexes, and drop unused and duplicate indexes elsewhere#5118connortechnology wants to merge 2 commits into
Conversation
…, FrameId) Every query against Frames is anchored on EventId, and nearly all of them order or range by FrameId within the event: the full-event reads in zm_eventstream.cpp and zma.cpp, the prev/next bulk frame LIMIT 1 lookups in includes/Event.php and ajax/status.php, zmfilter.pl's per-type scan, and the per-event aggregates in Event.pm and zmaudit.pl. A plain EventId index left all of those to a filesort. For the LIMIT 1 lookups on the playback path that meant reading and sorting an entire event's frames to return one row. Measured on a 200 event x 500 frame table, the prev-frame query goes from "ref EventId_idx, rows 500, Using filesort" to "range EventId_FrameId_idx, Using index condition, Backward index scan", the full-event ORDER BY FrameId loses its filesort, and max(FrameId) WHERE EventId becomes "Select tables optimized away". Nothing filters or sorts on Type or TimeStamp without EventId, so neither index was ever used for reading. Type is a three-value enum and cannot be selective in any case. Both only cost insert time on the highest-insert-rate table in the schema, plus space, plus work on every DELETE ... WHERE EventId. The migration adds the composite index before dropping EventId_idx so that its leftmost prefix covers EventId for any install still carrying the foreign key on Frames.EventId that 1.35.11 added and 1.37.31 drops only when it is named Frames_ibfk_1. Verified against a schema with that foreign key present: the drop succeeds and the constraint survives. Verified idempotent by running it twice. The index is left non-unique. FrameId is unique within an event, but a UNIQUE constraint would turn a duplicate-id bug into frames dropped mid-recording rather than a log line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An audit of every table in zm_create.sql.in against the queries that read it turned up seven indexes that are either never used or duplicate another index. Logs.TimeKey duplicates Logs_TimeKey_idx, which zm_create.sql.in creates three lines below it and zm_update-1.31.11.sql adds to upgraded installs. Both have existed on every install since, and every log INSERT maintains both. Logs is the second highest insert-rate table in the schema. Stats.MonitorId and Stats.ZoneId are never read. Every query against Stats is anchored on EventId: the per-frame zone stats view in skins functions.php and ajax/stats.php, the ZoneId filter term in FilterTerm.php and Filter.pm, the deletes in Event.php, Event.pm and zmaudit.pl, and the orphan scan. EventId_ZoneId serves all of them. A Stats row is written per frame per zone when ZM_RECORD_EVENT_STATS is on, which is the same insert-rate argument as the Frames indexes in the previous commit. EncoderTemplates.Encoder is the leftmost prefix of Encoder_Name, and the RoleId indexes on Role_Groups_Permissions and Role_Monitors_Permissions are the leftmost prefix of the UNIQUE index beside them. Those tables are small and rarely written, so this is tidying rather than a saving. Monitor_Status_UpdatedOn_idx is dropped by zm_update-1.37.76.sql but was left in zm_create.sql.in, so fresh installs have carried it and upgraded installs have not. Removed from zm_create.sql.in and repeated in the migration so the two agree from here on. Stats.MonitorId and Stats.ZoneId are the only two with no other index covering their column, so their drop is guarded on there being no foreign key on the column. 1.37.31 drops the Stats foreign keys only when they are named Stats_ibfk_1..4; on an install where they survived under another name the drop would fail with errno 150 and abort the upgrade, so it skips with a message instead. Verified by building that case and confirming the migration completes. Tested against MySQL 8.4: the schema built from master's zm_create.sql.in, with 1.39.27 and 1.39.28 applied, is index for index identical to a fresh install from the updated zm_create.sql.in - 123 indexes across 55 tables. Both migrations verified idempotent, and all 29 foreign keys survive. Not changed: AI_Detections carries three single-column indexes and has no consumer anywhere in the tree yet, so there is nothing to judge them against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Added a second commit (
The End-to-end test: master's Checked and left alone: |
There was a problem hiding this comment.
🟡 Changes recommended
The PR metadata (title/description) does not match the actual version/migration scope (includes 1.39.28 and additional index drops), which should be reconciled before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the ZoneMinder database schema to reduce write overhead and improve query performance by consolidating and removing redundant secondary indexes (most notably on the high-write Frames table), while keeping fresh installs and upgraded installs aligned.
Changes:
- Add
EventId_FrameId_idx (EventId, FrameId)toFramesand drop the previousEventId,Type, andTimeStampsecondary indexes viazm_update-1.39.27.sql. - Drop several redundant/unused indexes on
Logs,Stats,EncoderTemplates, role permission tables, andMonitor_Statusviazm_update-1.39.28.sql, matchingzm_create.sql.in. - Bump the schema version to
1.39.28.
File summaries
| File | Description |
|---|---|
| version.txt | Bumps schema version to 1.39.28. |
| db/zm_update-1.39.27.sql | Adds composite Frames(EventId, FrameId) index and drops redundant Frames secondary indexes. |
| db/zm_update-1.39.28.sql | Drops additional redundant/unused indexes across several tables (idempotent/guarded). |
| db/zm_create.sql.in | Updates fresh-install schema to match the post-migration index set. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -1 +1 @@ | |||
| 1.39.26 | |||
| 1.39.28 | |||
|
Fair catch on the metadata — the title and description were written for the Frames change alone and were never updated when No code change; 1.39.27 and 1.39.28 are both intended, one per logical change so either can be reverted on its own. |
Two migrations, from an audit of every table in
zm_create.sql.inagainst the queries that actually read it.version.txtgoes to 1.39.28.db/zm_update-1.39.27.sql— replace the threeFramessecondary indexes with one composite.db/zm_update-1.39.28.sql— drop seven indexes elsewhere that are never read or duplicate another index.1.39.27 — Frames
Framescarries three secondary indexes:EventId_idx (EventId),Type (Type)andTimeStamp (TimeStamp). These become a singleEventId_FrameId_idx (EventId, FrameId).Access patterns
Every query against
Framesis anchored onEventId, and nearly all of them order or range byFrameIdwithin the event:src/zm_eventstream.cpp:281,src/zma.cpp:418WHERE EventId=? ORDER BY FrameId ASCweb/includes/Event.php:770,774,web/ajax/status.php:513,515WHERE EventId=? AND FrameId </> ? ORDER BY FrameId LIMIT 1web/includes/Event.php:368,web/includes/functions.php:841WHERE EventId=? AND Score=? ORDER BY FrameId LIMIT 1scripts/zmfilter.pl.inWHERE EventId=? AND Type=? ORDER BY FrameIdZoneMinder/Event.pm:1062,scripts/zmaudit.pl.in:775max(TimeStamp), max(FrameId), sum(Score) WHERE EventId=?src/zm_eventstream.cpp:438,web/ajax/status.php:187-190min/max(Delta),min/max(FrameId)per eventweb/api(/frames/index/EventId:N, montage review)WHERE EventId IN (...)The plain
EventIdindex left all of those to a filesort. For theLIMIT 1prev/next-frame lookups on the playback path, that meant reading and sorting an entire event's frames to return one row.Nothing filters or sorts on
TypeorTimeStampwithoutEventId, so neither index was ever used for reading.Typeis a three-value enum and cannot be selective in any case. Both only cost insert time on the highest-insert-rate table in the schema, plus space, plus work on everyDELETE ... WHERE EventId.Measured
200 events x 500 frames, same data in a table with the old indexes and one with the new, MySQL 8.4:
WHERE EventId=? AND FrameId < ? ORDER BY FrameId DESC LIMIT 1ref EventId_idx, rows 500, Using filesortrange, Using index condition; Backward index scanWHERE EventId=? ORDER BY FrameIdmax(FrameId) WHERE EventId=?Ordering
The composite index is added before
EventId_idxis dropped, so that its leftmost prefix coversEventIdfor any install still carrying the foreign key onFrames.EventIdthat 1.35.11 added and 1.37.31 drops only when it happens to be namedFrames_ibfk_1. Dropping first would fail on those. Tested against a schema with that foreign key present: the drop succeeds and the constraint survives.The index is deliberately not UNIQUE.
FrameIdis unique within an event — it was the primary key before 1.28.107 — but a UNIQUE constraint would turn a duplicate-id bug into frames dropped mid-recording rather than a log line.1.39.28 — the rest of the schema
Logs.TimeKeyLogs_TimeKey_idx, created three lines below it inzm_create.sql.inand added to upgraded installs by 1.31.11. Both present everywhere, and every log INSERT maintains both.Logsis the second highest insert-rate table in the schema.Stats.MonitorId,Stats.ZoneIdStatsis anchored onEventId— the per-frame zone stats view, the ZoneId filter term inFilterTerm.phpandFilter.pm, the deletes inEvent.php,Event.pmandzmaudit.pl— andEventId_ZoneIdserves them all. A row is written per frame per zone whenZM_RECORD_EVENT_STATSis on, so this is the same insert-rate argument as theFramesindexes above.EncoderTemplates.EncoderEncoder_Name.Role_Groups_Permissions_RoleId_idx,Role_Monitors_Permissions_RoleId_idxMonitor_Status_UpdatedOn_idxzm_update-1.37.76.sqlbut left inzm_create.sql.in, so fresh installs have carried it and upgraded installs have not. Removed fromzm_create.sql.inand repeated in the migration so the two agree from here on.Stats.MonitorIdandStats.ZoneIdare the only two with no other index covering their column, so their drop is guarded on there being no foreign key on it. 1.37.31 drops theStatsforeign keys only when they are namedStats_ibfk_1..4; where one survived under another name the drop would fail with errno 150 and abort the upgrade. Built that case and confirmed it skips with a message instead.Testing
Against MySQL 8.4:
zm_create.sql.in, with 1.39.27 and 1.39.28 applied, is index for index identical to a fresh install from the updatedzm_create.sql.in— 123 indexes across 55 tables, all 29 foreign keys intact.Framesforeign-key case and theStatsforeign-key case each built explicitly and confirmed.Notes
On a
Framestable with hundreds of millions of rows theADD INDEXis an InnoDB in-place build: it does not block writes, but it will take a while. The migration prints a warning before starting. Every drop is effectively instant.The per-event aggregates (
sum(Score),max(TimeStamp),min/max(Delta)) still read every row of the event. Covering them would need all six columns in the index, i.e. a second copy of the table, so they are left alone.Checked and left as they are:
Events(all four indexes map to real query shapes),Events_Tags(theON E.Id = ET.EventIdjoin in the events list is covered by theEvents_Tags_ibfk_2foreign key index),Server_Stats(both indexes used), and the small tables.AI_Detectionscarries three single-column indexes and has no consumer anywhere in the tree yet, so there is nothing to judge them against — left for whenever that feature lands.🤖 Generated with Claude Code