Skip to content

Commit dfe6763

Browse files
committed
fix: improve report completeness and resource calculations
Preserve detailed auto_explain logs with larger capture and transport budgets, retain partial results, and redact credentials in stored query text. Correct table and slot deltas, security checks for unanalyzed tables, wait-series identity, sampled ClientWrite scoring, and log-window boundaries. Remove duplicate plan-chart boundary points and expand regression coverage.
1 parent 2ae2fd3 commit dfe6763

39 files changed

Lines changed: 411 additions & 42 deletions

‎README.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,6 +1115,13 @@ block opens its sanitized, bounded plan in the bundled read-only
11151115
`pg-explain-viewer`. An SFTP fallback for shell-less SSH
11161116
accounts is planned but deliberately not part of this release.
11171117

1118+
Log collection is bounded to 512 MiB scanned and 128 MiB returned per window.
1119+
An auto_explain record can retain up to 512 KiB; chart plan references have a
1120+
separate 64 MiB / 1024-plan budget. Reaching a scan or return limit preserves
1121+
already collected events and marks coverage incomplete; it does not discard the
1122+
whole item. Larger limits allow detailed analytical plans while keeping resource
1123+
use bounded.
1124+
11181125
The artifact records the phase outcome in `runtime.log_collection`
11191126
(`{status, reason, coverage}`); `coverage` states the requested and actually
11201127
covered window, scanned bytes, and truncation reasons, so an incomplete window

‎src/pg_diag/artifact.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from .security import (
2222
json_safe,
2323
redact_error,
24+
redact_query_credentials,
2425
redact_text,
2526
sanitize_public_structure,
2627
sanitize_result,
@@ -461,7 +462,7 @@ def _remember_query_text(query_texts: dict[str, str], query_id: Any, query_text:
461462
if query_id is None or query_text is None:
462463
return
463464
query_id_text = str(query_id).strip()
464-
sql_text = str(query_text).strip()
465+
sql_text = redact_query_credentials(str(query_text).strip())
465466
if not query_id_text or not sql_text:
466467
return
467468
existing = query_texts.get(query_id_text)

‎src/pg_diag/cli.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,7 +751,9 @@ def _machine_artifacts(args: argparse.Namespace) -> list[dict[str, Any]]:
751751
args.output_format,
752752
)
753753
if json_path is not None:
754-
paths.append(("DiagnosticReport", "pg_diag/artifact-v4", json_path))
754+
paths.append(
755+
("DiagnosticReport", f"pg_diag/artifact-v{runtime_config.ARTIFACT_SCHEMA_VERSION}", json_path)
756+
)
755757
if html_path is not None:
756758
paths.append(("DiagnosticReportHtml", None, html_path))
757759
artifacts = []

‎src/pg_diag/content/instructions/items/snapshot_delta_workload/logical_decoding_slot_delta.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ This instruction belongs to report item `snapshot_delta_workload.logical_decodin
1212
- No severity is assigned because spilling can be acceptable for large transactions and bounded workloads.
1313

1414
## Interval coverage
15-
- Slot name is the stable key and `stats_reset` is the reset epoch.
16-
- New, dropped, or reset slots are omitted rather than represented as zero deltas.
15+
- Slot name identifies the endpoint pair and `stats_reset` detects explicit statistics resets.
16+
- Slots absent or already lost at either endpoint, changed reset epochs, and decreasing counters are omitted rather than represented as zero deltas.
17+
- PostgreSQL does not expose a slot creation identifier here. Dropping and recreating a healthy slot with the same name between endpoints can remain undetectable when `stats_reset` is NULL and counters do not decrease. Check lifecycle logs before interpreting such a window.
1718

1819
## Common fault causes
1920
- Large transactions, insufficient `logical_decoding_work_mem`, slow consumers, and output-plugin behavior.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
sha256:b7599f0f621cdb60f871af8c3155d98d52e3f97aa17de2b218549ec6d74f1a7d
1+
sha256:19e60f96138c7a86a240fefa473d881711d1137a803fd119fd37baa0fd8ca16b

‎src/pg_diag/content/metrics.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1918,7 +1918,7 @@ metrics:
19181918
mode: interval
19191919
limit: 12
19201920
key_refs: [dimensions.database, dimensions.wait_event_type, dimensions.wait_event, dimensions.query_id]
1921-
label_refs: [dimensions.wait_event_type, dimensions.wait_event, dimensions.query_id]
1921+
label_refs: [dimensions.wait_event_type, dimensions.wait_event, dimensions.query_id, dimensions.database]
19221922
value_ref: gauges.sessions
19231923
transform: avg
19241924
unit: sessions

‎src/pg_diag/content/python/backend/postgres_main_process_linked_libraries.py‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,15 @@
5656
esac
5757
5858
case "$postgres_main_pid" in
59-
''|*[!0-9]*|0|1)
59+
''|*[!0-9]*|0)
6060
echo "cannot resolve the parent of PostgreSQL backend PID $backend_pid" >&2
6161
exit 43
6262
;;
6363
esac
6464
6565
postgres_main_status=/proc/$postgres_main_pid/status
66+
# In a container the postmaster can be PID 1. Validate its process name below
67+
# just as for any other parent, rather than rejecting the namespace init PID.
6668
if [ ! -r "$postgres_main_status" ]; then
6769
echo "parent PID $postgres_main_pid of PostgreSQL backend PID $backend_pid is not visible" >&2
6870
exit 43
@@ -89,6 +91,16 @@
8991
esac
9092
9193
postgres_executable=$(readlink "/proc/$postgres_main_pid/exe" 2>/dev/null || true)
94+
postgres_owner=
95+
runuser_bin=$(command -v runuser 2>/dev/null || true)
96+
if [ -z "$postgres_executable" ] && [ "$(id -u)" = 0 ] && [ -n "$runuser_bin" ]; then
97+
# Container root may lack CAP_SYS_PTRACE. The actual process owner can still
98+
# inspect its executable; use that identity only for readlink and ldd.
99+
postgres_owner=$(stat -c %U "$postgres_main_status" 2>/dev/null || true)
100+
if [ -n "$postgres_owner" ]; then
101+
postgres_executable=$("$runuser_bin" -u "$postgres_owner" -- readlink "/proc/$postgres_main_pid/exe" 2>/dev/null || true)
102+
fi
103+
fi
92104
if [ -z "$postgres_executable" ]; then
93105
echo "cannot resolve /proc/$postgres_main_pid/exe for PostgreSQL backend PID $backend_pid" >&2
94106
exit 45
@@ -105,7 +117,11 @@
105117
printf 'PGDIAG_POSTGRES_EXECUTABLE=%s\n' "$postgres_executable"
106118
printf 'PGDIAG_LDD_PATH=%s\n' "$ldd_bin"
107119
printf '%s\n' 'PGDIAG_LDD_BEGIN'
108-
"$ldd_bin" "/proc/$postgres_main_pid/exe"
120+
if [ -n "$postgres_owner" ]; then
121+
"$runuser_bin" -u "$postgres_owner" -- "$ldd_bin" "/proc/$postgres_main_pid/exe"
122+
else
123+
"$ldd_bin" "/proc/$postgres_main_pid/exe"
124+
fi
109125
"""
110126

111127
_RESOLVED_RE = re.compile(

‎src/pg_diag/content/python/server_log/auto_explain_plans.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,11 @@ def collect(context: PythonSourceContext) -> PythonSourceResult:
8282
if rank < len(top_by_bucket[bucket]) and displayed_plan_count < event_point_limit:
8383
selected[bucket][rank] = top_by_bucket[bucket][rank]
8484
displayed_plan_count += 1
85+
occupied_first_rank = {
86+
_iso_timestamp(bucket, utc_offset_seconds)
87+
for bucket, ranks in selected.items() if 0 in ranks
88+
}
89+
axis_points = [point for point in axis_points if point["t"] not in occupied_first_rank]
8590
refs = ChartReferencePool()
8691

8792
result = {

‎src/pg_diag/content/queries/metrics/logical_decoding_slot_delta.sql‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ select
1010
stream_bytes::int8 as stream_bytes,
1111
total_txns::int8 as total_txns,
1212
total_bytes::int8 as total_bytes
13-
from pg_catalog.pg_stat_replication_slots
13+
from pg_catalog.pg_stat_replication_slots s
14+
join pg_catalog.pg_replication_slots r using (slot_name)
15+
-- A lost slot cannot resume decoding. Reusing its name after recreation does
16+
-- not reliably change stats_reset, so its counters are not a valid endpoint.
17+
where r.wal_status is distinct from 'lost'
1418
order by spill_bytes desc nulls last, slot_name
1519
limit 50

‎src/pg_diag/content/queries/metrics/objects_table_scan_delta.sql‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ select
99
as database_stats_reset,
1010
st.seq_scan::int8 as seq_scan,
1111
st.seq_tup_read::int8 as seq_tup_read,
12-
st.idx_scan::int8 as idx_scan,
13-
st.idx_tup_fetch::int8 as idx_tup_fetch
12+
-- pg_stat_all_tables sums index scans; the sum is NULL when no indexes exist.
13+
coalesce(st.idx_scan, 0)::int8 as idx_scan,
14+
coalesce(st.idx_tup_fetch, 0)::int8 as idx_tup_fetch
1415
from pg_stat_all_tables st
1516
join pg_class c on c.oid = st.relid
1617
join pg_namespace n on n.oid = c.relnamespace

0 commit comments

Comments
 (0)