mirror of
https://github.com/domainaware/parsedmarc.git
synced 2026-10-23 19:18:58 +00:00
Fix "Aggregate Report Details" Grafana panel timing out on PostgreSQL (#923)
PostgreSQL does not create an index on the referencing side of a foreign key (docs, "Foreign Keys"), and create_tables() indexed only two of the schema's seven foreign-key columns. The panel's per-record DKIM/SPF subqueries on record_id therefore ran two sequential scans per row and timed out on a 413k-record database. Index the five remaining foreign-key columns, and add a test that every REFERENCES column in the schema has an index. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
4d3afac0c9
commit
65a7bfb7f9
3 files changed
+67
No files matched your search
@@ -1,5 +1,11 @@
|
||||
# Changelog
|
||||
|
||||
## Unreleased
|
||||
|
||||
### Bug fixes
|
||||
|
||||
- **The PostgreSQL schema now indexes its foreign-key columns, so the PostgreSQL Grafana dashboard's "Aggregate Report Details" table no longer times out.** PostgreSQL indexes the primary key a `REFERENCES` column points at, but not the referencing column itself ("the declaration of a foreign key constraint does not automatically create an index on the referencing columns", [PostgreSQL docs, "Foreign Keys"](https://www.postgresql.org/docs/current/ddl-constraints.html#DDL-CONSTRAINTS-FK)), and `create_tables()` indexed only two of the schema's seven foreign-key columns (`dmarc_aggregate_record.report_id` and `smtp_tls_policy.report_id`). The panel looks up each record's DKIM and SPF results with a correlated subquery on `dmarc_aggregate_record_dkim.record_id` and `dmarc_aggregate_record_spf.record_id`, so every row of the panel ran two sequential scans of those tables. With 413,237 aggregate records (5.5 years of reports, PostgreSQL 16), 500 records took 46.1 s, about 92 ms each; a one-year time range of 43,568 records would have taken over an hour, and Grafana returned a 504 long before that. `create_tables()` now also creates `idx_agg_record_dkim_record_id`, `idx_agg_record_spf_record_id`, `idx_agg_record_override_record_id`, `idx_failure_sample_address_report_id`, and `idx_smtp_tls_failure_detail_policy_id`, covering every foreign-key column in the schema; the same 500 records take 15.8 ms and the one-year panel query 3.2 s. `create_tables()` runs on every start with `CREATE INDEX IF NOT EXISTS`, so existing databases get the indexes on their next start; the two largest indexes took about 0.3 s each to build on the database above, and the indexed tables reject writes until the startup DDL transaction commits.
|
||||
|
||||
## 11.0.4
|
||||
|
||||
### Security
|
||||
|
||||
@@ -428,6 +428,28 @@ class PostgreSQLClient:
|
||||
CREATE INDEX IF NOT EXISTS idx_smtp_tls_policy_report_id
|
||||
ON smtp_tls_policy (report_id)
|
||||
""",
|
||||
# ----- remaining foreign-key columns (PostgreSQL does not index
|
||||
# the referencing side) -----
|
||||
"""
|
||||
CREATE INDEX IF NOT EXISTS idx_agg_record_dkim_record_id
|
||||
ON dmarc_aggregate_record_dkim (record_id)
|
||||
""",
|
||||
"""
|
||||
CREATE INDEX IF NOT EXISTS idx_agg_record_spf_record_id
|
||||
ON dmarc_aggregate_record_spf (record_id)
|
||||
""",
|
||||
"""
|
||||
CREATE INDEX IF NOT EXISTS idx_agg_record_override_record_id
|
||||
ON dmarc_aggregate_record_policy_override (record_id)
|
||||
""",
|
||||
"""
|
||||
CREATE INDEX IF NOT EXISTS idx_failure_sample_address_report_id
|
||||
ON dmarc_failure_sample_address (report_id)
|
||||
""",
|
||||
"""
|
||||
CREATE INDEX IF NOT EXISTS idx_smtp_tls_failure_detail_policy_id
|
||||
ON smtp_tls_failure_detail (policy_id)
|
||||
""",
|
||||
]
|
||||
|
||||
try:
|
||||
|
||||
@@ -337,6 +337,45 @@ class TestPostgreSQLConstruction(unittest.TestCase):
|
||||
self.assertIn(f"CREATE TABLE IF NOT EXISTS {table}", executed)
|
||||
self.assertIn("CREATE INDEX IF NOT EXISTS", executed)
|
||||
|
||||
def test_create_tables_indexes_every_foreign_key(self):
|
||||
"""Every REFERENCES column gets an index.
|
||||
|
||||
PostgreSQL indexes the referenced primary key but not the referencing
|
||||
column ("the declaration of a foreign key constraint does not
|
||||
automatically create an index on the referencing columns", PostgreSQL
|
||||
docs, "Foreign Keys"), so without one a per-record lookup such as the
|
||||
PostgreSQL Grafana dashboard's DKIM/SPF result subqueries scans the
|
||||
whole child table.
|
||||
"""
|
||||
import re
|
||||
|
||||
client, mock_conn = _make_client()
|
||||
cur = _mock_cursor(mock_conn, [])
|
||||
client.create_tables()
|
||||
statements = _executed_sql(cur)
|
||||
|
||||
foreign_keys = set()
|
||||
for stmt in statements:
|
||||
table = re.search(r"CREATE TABLE IF NOT EXISTS (\w+)", stmt)
|
||||
if table:
|
||||
for column in re.findall(
|
||||
r"^\s*(\w+)[^\n]*\n?\s*REFERENCES", stmt, re.M | re.I
|
||||
):
|
||||
foreign_keys.add((table.group(1), column))
|
||||
indexed = {
|
||||
(m.group(1), m.group(2))
|
||||
for stmt in statements
|
||||
if (m := re.search(r"ON (\w+) \((\w+)\)", stmt))
|
||||
}
|
||||
|
||||
# Every REFERENCES in the DDL must have been attributed to a column,
|
||||
# so a layout the column regex misses fails here instead of passing.
|
||||
references = sum(
|
||||
len(re.findall(r"\bREFERENCES\b", s, re.I)) for s in statements
|
||||
)
|
||||
self.assertEqual(len(foreign_keys), references)
|
||||
self.assertEqual(foreign_keys - indexed, set())
|
||||
|
||||
def test_create_tables_wraps_db_error(self):
|
||||
"""A driver error during DDL is wrapped in PostgreSQLError."""
|
||||
|
||||
|
||||
Reference in new issue
Block a user