Skip to content

Commit 2f3c31f

Browse files
committed
fix: add time-based filtering and retention to /query_info_metrics export
PostgREST generates unique queryids at ~7K/min, causing pgss_queryid_queries to grow unboundedly (520K+ rows/hour). The /query_info_metrics endpoint scans all rows with DISTINCT ON, exceeding the 30s VictoriaMetrics scrape timeout and breaking Dashboard 2's group_left() join. Three-part fix: - Trigger: UPDATE time on duplicate instead of silently skipping, so active queryids (still in pg_stat_statements) have recent timestamps - Time filter: only export queryids seen in the last 10 minutes (~5K rows instead of 520K), well within the 30s scrape timeout - Retention cleanup: delete entries older than 24h in batches of 10K per scrape to prevent unbounded table growth Also applies trigger migration at Flask startup (idempotent) for existing deployments, and adds time filtering to get_query_texts_from_sink(). https://claude.ai/code/session_01SzJxzZNQjDQphaHyaX3RU7
1 parent b726f7d commit 2f3c31f

3 files changed

Lines changed: 152 additions & 25 deletions

File tree

config/sink-postgres/init.sql

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,29 +55,31 @@ declare
5555
begin
5656
-- Extract queryid from the data JSONB
5757
queryid_value := new.data->>'queryid';
58-
58+
5959
-- Allow NULL queryids through
6060
if queryid_value is null then
6161
return new;
6262
end if;
63-
64-
-- Silently skip if duplicate exists
65-
if exists (
66-
select
67-
from pgss_queryid_queries
68-
where
69-
dbname = new.dbname
70-
and data->>'queryid' = queryid_value
71-
limit 1
72-
) then
73-
return null; -- Cancels INSERT silently
63+
64+
-- If duplicate exists, refresh its timestamp (marks queryid as still active
65+
-- in pg_stat_statements) and skip the INSERT
66+
update pgss_queryid_queries
67+
set time = new.time
68+
where dbname = new.dbname
69+
and data->>'queryid' = queryid_value;
70+
71+
if found then
72+
return null; -- Duplicate updated, cancel INSERT
7473
end if;
75-
76-
return new;
74+
75+
return new; -- New queryid, proceed with INSERT
7776
end;
7877
$$ language plpgsql;
7978

8079

80+
-- Allow pgwatch to update this function (Flask app applies trigger migrations at startup)
81+
alter function enforce_queryid_uniqueness() owner to pgwatch;
82+
8183
create or replace trigger enforce_queryid_uniqueness_trigger
8284
before insert
8385
on pgss_queryid_queries

monitoring_flask_backend/app.py

Lines changed: 102 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,68 @@ def smart_truncate_query(query: str, max_length: int = 40) -> str:
187187
# Prometheus connection - use environment variable with fallback
188188
PROMETHEUS_URL = os.environ.get('PROMETHEUS_URL', 'http://localhost:8428')
189189

190+
# Retention: how long to keep stale queryid entries (queryids no longer in pg_stat_statements)
191+
QUERYID_RETENTION_HOURS = int(os.environ.get('QUERYID_RETENTION_HOURS', '24'))
192+
193+
# How recent a queryid must be to appear in /query_info_metrics (must be > pgwatch collection interval)
194+
QUERYID_ACTIVE_MINUTES = int(os.environ.get('QUERYID_ACTIVE_MINUTES', '10'))
195+
196+
197+
def _apply_trigger_migration():
198+
"""
199+
Apply trigger migration: update queryid timestamps on duplicate instead of
200+
silently skipping. This lets us distinguish active queryids (recently seen in
201+
pg_stat_statements) from stale ones, enabling time-based filtering and retention.
202+
203+
Safe to call multiple times (CREATE OR REPLACE FUNCTION is idempotent).
204+
"""
205+
conn = None
206+
try:
207+
conn = psycopg2.connect(POSTGRES_SINK_URL)
208+
conn.autocommit = True
209+
with conn.cursor() as cursor:
210+
cursor.execute("""
211+
CREATE OR REPLACE FUNCTION enforce_queryid_uniqueness()
212+
RETURNS trigger AS $$
213+
DECLARE
214+
queryid_value text;
215+
BEGIN
216+
queryid_value := new.data->>'queryid';
217+
218+
IF queryid_value IS NULL THEN
219+
RETURN new;
220+
END IF;
221+
222+
-- If duplicate exists, refresh its timestamp (marks queryid as
223+
-- still active in pg_stat_statements) and skip the INSERT
224+
UPDATE pgss_queryid_queries
225+
SET time = new.time
226+
WHERE dbname = new.dbname
227+
AND data->>'queryid' = queryid_value;
228+
229+
IF FOUND THEN
230+
RETURN NULL;
231+
END IF;
232+
233+
RETURN new;
234+
END;
235+
$$ LANGUAGE plpgsql;
236+
""")
237+
logger.info("Applied trigger migration: enforce_queryid_uniqueness now refreshes timestamps")
238+
except Exception as e:
239+
logger.warning(
240+
f"Failed to apply trigger migration: {e}. "
241+
"If 'must be owner' error, run with postgres superuser: "
242+
"ALTER FUNCTION enforce_queryid_uniqueness() OWNER TO pgwatch;"
243+
)
244+
finally:
245+
if conn:
246+
conn.close()
247+
248+
249+
# Apply trigger migration at startup (idempotent)
250+
_apply_trigger_migration()
251+
190252
# Metric name mapping for cleaner CSV output
191253
METRIC_NAME_MAPPING = {
192254
'calls': 'calls',
@@ -230,17 +292,22 @@ def get_prometheus_client():
230292
raise
231293

232294

233-
def get_query_texts_from_sink(db_name: str = None, truncation_mode: str = 'smart') -> dict:
295+
def get_query_texts_from_sink(db_name: str = None, truncation_mode: str = 'smart',
296+
max_age_hours: int = None) -> dict:
234297
"""
235298
Fetch queryid-to-query text mappings from the PostgreSQL sink database.
236299
237300
Args:
238301
db_name: Optional database name to filter results
239302
truncation_mode: 'smart' for smart truncation, 'raw' for simple truncation
303+
max_age_hours: Only return queryids seen within this many hours (None = use retention window)
240304
241305
Returns:
242306
Dictionary mapping queryid to query text
243307
"""
308+
if max_age_hours is None:
309+
max_age_hours = QUERYID_RETENTION_HOURS
310+
244311
query_texts = {}
245312

246313
conn = None
@@ -257,23 +324,25 @@ def get_query_texts_from_sink(db_name: str = None, truncation_mode: str = 'smart
257324
FROM public.pgss_queryid_queries
258325
WHERE
259326
dbname = %s
327+
AND time > now() - interval '%s hours'
260328
AND data->>'queryid' IS NOT NULL
261329
AND data->>'query' IS NOT NULL
262330
ORDER BY data->>'queryid', time DESC
263331
"""
264-
cursor.execute(query, (db_name,))
332+
cursor.execute(query, (db_name, max_age_hours))
265333
else:
266334
query = """
267335
SELECT DISTINCT ON (data->>'queryid')
268336
data->>'queryid' as queryid,
269337
data->>'query' as query
270338
FROM public.pgss_queryid_queries
271339
WHERE
272-
data->>'queryid' IS NOT NULL
340+
time > now() - interval '%s hours'
341+
AND data->>'queryid' IS NOT NULL
273342
AND data->>'query' IS NOT NULL
274343
ORDER BY data->>'queryid', time DESC
275344
"""
276-
cursor.execute(query)
345+
cursor.execute(query, (max_age_hours,))
277346

278347
for row in cursor:
279348
queryid = row['queryid']
@@ -1284,7 +1353,28 @@ def get_query_info_metrics():
12841353
try:
12851354
conn = psycopg2.connect(POSTGRES_SINK_URL)
12861355
with conn.cursor(cursor_factory=psycopg2.extras.DictCursor) as cursor:
1287-
# Skip db_name filter if it's empty, "All", or contains special chars
1356+
# Retention cleanup: delete stale entries in batches to prevent
1357+
# unbounded table growth from high-churn queryid sources (e.g. PostgREST)
1358+
try:
1359+
cursor.execute("""
1360+
DELETE FROM public.pgss_queryid_queries
1361+
WHERE ctid IN (
1362+
SELECT ctid FROM public.pgss_queryid_queries
1363+
WHERE time < now() - interval '%s hours'
1364+
LIMIT 10000
1365+
)
1366+
""", (QUERYID_RETENTION_HOURS,))
1367+
deleted = cursor.rowcount
1368+
conn.commit()
1369+
if deleted > 0:
1370+
logger.info(f"Retention cleanup: deleted {deleted} stale queryid entries")
1371+
except Exception as e:
1372+
logger.warning(f"Retention cleanup failed: {e}")
1373+
conn.rollback()
1374+
1375+
# Only export queryids recently seen in pg_stat_statements.
1376+
# The dedup trigger refreshes the timestamp on each collection,
1377+
# so active queryids have time within the last few minutes.
12881378
use_db_filter = db_name and db_name.lower() not in ('all', '') and not db_name.startswith('$')
12891379
if use_db_filter:
12901380
query = """
@@ -1294,23 +1384,25 @@ def get_query_info_metrics():
12941384
FROM public.pgss_queryid_queries
12951385
WHERE
12961386
dbname = %s
1387+
AND time > now() - interval '%s minutes'
12971388
AND data->>'queryid' IS NOT NULL
12981389
AND data->>'query' IS NOT NULL
12991390
ORDER BY data->>'queryid', time DESC
13001391
"""
1301-
cursor.execute(query, (db_name,))
1392+
cursor.execute(query, (db_name, QUERYID_ACTIVE_MINUTES))
13021393
else:
13031394
query = """
13041395
SELECT DISTINCT ON (data->>'queryid')
13051396
data->>'queryid' as queryid,
13061397
data->>'query' as query
13071398
FROM public.pgss_queryid_queries
13081399
WHERE
1309-
data->>'queryid' IS NOT NULL
1400+
time > now() - interval '%s minutes'
1401+
AND data->>'queryid' IS NOT NULL
13101402
AND data->>'query' IS NOT NULL
13111403
ORDER BY data->>'queryid', time DESC
13121404
"""
1313-
cursor.execute(query)
1405+
cursor.execute(query, (QUERYID_ACTIVE_MINUTES,))
13141406

13151407
for row in cursor:
13161408
queryid = row['queryid']
@@ -1355,6 +1447,8 @@ def get_query_info_metrics():
13551447
'full': full,
13561448
'queryid_only': queryid_only,
13571449
}
1450+
1451+
logger.info(f"Exported {len(query_data)} active queryids for metrics")
13581452
except Exception as e:
13591453
logger.warning(f"Failed to fetch query texts from sink database: {e}")
13601454
finally:

monitoring_flask_backend/test_app.py

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ def test_db_name_filter(self, mock_connect, client):
523523
response = client.get('/query_info_metrics?db_name=mydb')
524524
assert response.status_code == 200
525525

526-
# Verify the query was called with db_name filter
526+
# Verify the main query (last execute call) was called with db_name filter
527527
call_args = mock_cursor.execute.call_args
528528
assert call_args is not None
529529
query = call_args[0][0]
@@ -538,7 +538,7 @@ def test_db_name_all_skips_filter(self, mock_connect, client):
538538
response = client.get('/query_info_metrics?db_name=all')
539539
assert response.status_code == 200
540540

541-
# Verify the query was called without db_name filter
541+
# Verify the main query (last execute call) was called without db_name filter
542542
call_args = mock_cursor.execute.call_args
543543
query = call_args[0][0]
544544
assert 'dbname = %s' not in query
@@ -552,11 +552,42 @@ def test_db_name_variable_skips_filter(self, mock_connect, client):
552552
response = client.get('/query_info_metrics?db_name=$db_name')
553553
assert response.status_code == 200
554554

555-
# Verify the query was called without db_name filter
555+
# Verify the main query (last execute call) was called without db_name filter
556556
call_args = mock_cursor.execute.call_args
557557
query = call_args[0][0]
558558
assert 'dbname = %s' not in query
559559

560+
@patch('app.psycopg2.connect')
561+
def test_time_filter_applied(self, mock_connect, client):
562+
"""Test that the time filter is applied to only export active queryids."""
563+
mock_cursor = mock_connect.return_value.cursor.return_value.__enter__.return_value
564+
mock_cursor.__iter__ = lambda self: iter([])
565+
566+
response = client.get('/query_info_metrics')
567+
assert response.status_code == 200
568+
569+
# Verify the main query includes a time filter
570+
call_args = mock_cursor.execute.call_args
571+
query = call_args[0][0]
572+
assert "interval" in query
573+
assert "minutes" in query
574+
575+
@patch('app.psycopg2.connect')
576+
def test_retention_cleanup_runs(self, mock_connect, client):
577+
"""Test that retention cleanup DELETE runs before the main query."""
578+
mock_cursor = mock_connect.return_value.cursor.return_value.__enter__.return_value
579+
mock_cursor.__iter__ = lambda self: iter([])
580+
mock_cursor.rowcount = 0
581+
582+
response = client.get('/query_info_metrics')
583+
assert response.status_code == 200
584+
585+
# Verify that execute was called at least twice (cleanup + main query)
586+
assert mock_cursor.execute.call_count >= 2
587+
# First call should be the retention DELETE
588+
first_call_query = mock_cursor.execute.call_args_list[0][0][0]
589+
assert 'DELETE' in first_call_query
590+
560591
@patch('app.psycopg2.connect')
561592
def test_handles_db_connection_error(self, mock_connect, client):
562593
"""Test endpoint handles database connection errors gracefully."""

0 commit comments

Comments
 (0)