Skip to content

pg_lake_table: ANALYZE pg_lake catalogs to keep commit-time diff off the N² cliff#352

Merged
sfc-gh-okalaci merged 4 commits into
mainfrom
okalaci/analyze_threshold_gate
May 20, 2026
Merged

pg_lake_table: ANALYZE pg_lake catalogs to keep commit-time diff off the N² cliff#352
sfc-gh-okalaci merged 4 commits into
mainfrom
okalaci/analyze_threshold_gate

Conversation

@sfc-gh-okalaci

Copy link
Copy Markdown
Collaborator

Two commits, both gating the same problem: the commit-time diff query joins three lake_table catalogs that the same tx has been bulk-inserting into, but autovacuum can't run inside an open tx, so the planner's reltuples is stale (often 0 on a freshly created table) and it picks an N² nested loop on a query that's comfortably hash-joinable.

On 24k files in one tx that's hours.

  1. ANALYZE pg_lake catalogs before commit-time diff — adds a cheap SPI ANALYZE lake_table.files, lake_table.data_file_partition_values, lake_table.data_file_column_stats before GetDataFileMetadataOperations runs. Gated on at least one tracked relation actually changing data files, so DDL-only trackers don't pay it.

  2. gate commit-time ANALYZE on data-file change count — adds hidden GUC pg_lake_table.commit_time_analyze_threshold (default 1000) so small commits skip ANALYZE (its cost is non-incremental and amortizes badly on hot-loop workloads). DATA_FILE_REMOVE_ALL (e.g., TRUNCATE/DROP) forces ANALYZE regardless because that one op fans out to thousands of catalog deletes.

Bench, 60 × 400 = 24k files in one tx, local PG17 + MinIO:

                 main       this PR
COMMIT           2505.8 s   47.7 s     ~52x
LOAD (60 COPYs)   946.8 s   907.7 s    unchanged

sfc-gh-okalaci and others added 2 commits May 13, 2026 18:42
The commit-time diff in GetDataFileMetadataOperations joins three
pg_lake_table catalogs that the same transaction has been bulk-inserting
into during the load phase:

  - lake_table.files
  - lake_table.data_file_partition_values
  - lake_table.data_file_column_stats

Autovacuum cannot run inside an open transaction, so on a tx that has
loaded tens of thousands of files those catalogs' pg_class.reltuples is
whatever it was at tx start -- often 0 on a freshly created table. With
stale stats the planner can pick an N^2 nested-loop plan that doesn't
finish in any reasonable time. ANALYZE before the diff is cheap
insurance against that cliff. Large partitioned writes are the workload
where it matters most.

Gated on at least one tracked relation actually changing data files,
so DDL-only trackers don't pay the few-ms ANALYZE cost.

Empirical, 60 batches x 400 files = 24000 files in one tx, local PG17 +
MinIO:

  step               main       this commit
  -----------------+----------+-------------
  COMMIT             2505.8 s   49.9 s
  LOAD (60 COPYs)    946.7 s    949.7 s

Commit time drops ~50x; load is unaffected. Subsequent commits in this
series target the remaining commit-time hotspots.

Co-authored-by: Cursor <cursoragent@cursor.com>
Introduce pg_lake_table.commit_time_analyze_threshold (default 1000,
GUC_NO_SHOW_ALL | GUC_NOT_IN_SAMPLE, PGC_USERSET). The previous gate
fired EnsureFreshStatsForCommitTimeDiff() on any data-file change in
the transaction; ANALYZE is not incremental, so every small commit paid
the full resampling cost on three catalogs. A threshold lets small
commits skip ANALYZE while bulk loads still get fresh planner stats.

Two signals feed the gate:
  - dataFileChangeCount sums DATA_FILE_ADD and DATA_FILE_REMOVE ops
    across all tracked relations. Both directions rewrite the same
    catalogs the diff joins, so both count toward the threshold.
  - forceCommitTimeAnalyze is set on DATA_FILE_REMOVE_ALL. That single
    op maps to thousands of catalog deletes, so we force ANALYZE
    regardless of the threshold.

Set the GUC to 1 to restore the previous "ANALYZE on any change"
behavior, or to INT_MAX to disable the upfront ANALYZE.

Empirical, 60 batches x 400 files = 24000 files in one tx (the bench
in this series), local PG17 + MinIO -- the large-tx case is unaffected
because 24000 ops blow past the 1000 default:

  step               analyze-only   this commit
  -----------------+--------------+-------------
  COMMIT              49.9 s         47.7 s
  LOAD (60 COPYs)     949.7 s        907.7 s

Stress test, 100 sequential auto-commit COPYs of one parquet file each
(400 files per COPY, 40k files at end), same env:

  per-COPY ms      default GUC (1000)   GUC=1 (always)
  --------------+--------------------+----------------
  mean                11212               9903
  p50                  9762               9555
  p95                 20456              12534
  max                 58928              13040

GUC=1 wins on this small-tx workload (catalogs stay fresh as they grow,
no tail spikes from stale-stat replanning). The default of 1000 is the
conservative choice: cheap insurance against ANALYZE thrashing on hot-
loop workloads where each tx is much smaller than the catalogs it
joins. Users hitting either failure mode can move the GUC.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread pg_lake_table/src/init.c
NULL,
NULL);

DefineCustomIntVariable("pg_lake_table.commit_time_analyze_threshold",

@sfc-gh-okalaci sfc-gh-okalaci May 14, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1000 is arbitrary, a number that'd make the execution times manageable even if we get bad plans.

We could consider lowering this a bit, like to 500 or even lower. At that scale, the cost of analyze is becoming negligible. We could even drop the second commit, and do it whenever any data files change, but that sounded/felt a bit too much,

Any thoughts here?

@sfc-gh-okalaci

Copy link
Copy Markdown
Collaborator Author

COMMIT 2505.8 s 47.7 s ~52x

This provides the largest benefit. I had anecdotal cases where I had seen analyze fixing this problem.

@sfc-gh-mslot sfc-gh-mslot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything we should do about regular analyze settings on these tables?

@sfc-gh-okalaci

Copy link
Copy Markdown
Collaborator Author

Anything we should do about regular analyze settings on these tables?

I think we could either reduce autovacuum_analyze_scale_factor or in our VACUUM track last_autoanalyze and kick that in:

SELECT relname, last_autoanalyze, last_autovacuum                                                                            FROM pg_stat_user_tables                                                                                                                          WHERE relname = 'files' and schemaname = 'lake_table';
 relname |       last_autoanalyze       |        last_autovacuum        
---------+------------------------------+-------------------------------
 files   | 2026-05-14 18:38:08.65319+03 | 2026-05-14 18:38:08.206628+03
(1 row)

Time: 4.794 ms

Perhaps reducing autovacuum_analyze_scale_factor is a good start?

@sfc-gh-dachristensen

Copy link
Copy Markdown
Collaborator

Anything we should do about regular analyze settings on these tables?

You mean if a user has set table-level settings, this having an impact on the raw ANALYZE, or just if there are improvements to be made by setting some table-level settings ourselves?

sfc-gh-okalaci and others added 2 commits May 20, 2026 10:20
LoadColumnStatsForFiles (data_files_catalog.c) is called once at commit
time from GetDataFileMetadataOperations
(track_iceberg_metadata_changes.c) with the list of files added in the
transaction. For each (path, field_id) row returned by the SPI query
against lake_table.data_file_column_stats it has to find the matching
caller-owned TableDataFile* and append a DataFileColumnStats to its
stats.columnStats list.

The previous implementation walked the entire dataFiles list per result
row with strcmp:

    foreach(lc, dataFiles)
    {
        TableDataFile *dataFile = lfirst(lc);
        if (strcmp(dataFile->path, path) != 0)
            continue;
        ...
        break;
    }

That comment ("typically 1-2 entries per transaction") was true for
small interactive workloads but is dramatically wrong for any append-
heavy iceberg write: an Iceberg table partitioned on
(client_id, month(ordered_date)) over a year of data lands tens of
thousands of files per transaction, and each file has ~6 columns
worth of stats. The fill loop is then N files * (N * C) inner
iterations -- quadratic in the number of files.

Fix: the caller (GetDataFileMetadataOperations) already builds a
path -> TableDataFile hash for the diff. Thread that hash through
to LoadColumnStatsForFiles and dispatch each SPI row to its target
file with one hash_search(HASH_FIND).

Empirical, 60 batches x 400 files = 24000 files in one tx, local PG17
+ MinIO, on top of the previous ANALYZE + threshold commits:

  step               threshold-gate   this commit
  -----------------+-----------------+-------------
  COMMIT              47.7 s           3.5 s
  LOAD (60 COPYs)     907.7 s          481.2 s

The 14x commit drop is the targeted win; the load improvement is
system variance (this commit only touches commit-time code).
Subsequent commits target the remaining commit-time hotspots
(in_progress_files batch delete, data-file catalog bulk insert, and
the fast-path that bypasses the diff entirely).

Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the silent skip when hash_search(filesByPath) misses a path
returned by the column-stats SPI query (review on #353). That case
violates the invariant that WHERE path = ANY($2) matches the caller's
hash keys; treat it as ERRCODE_INTERNAL_ERROR with errdetail instead
of masking possible catalog/extension bugs.

Co-authored-by: Cursor <cursoragent@cursor.com>
@sfc-gh-okalaci

Copy link
Copy Markdown
Collaborator Author

Anything we should do about regular analyze settings on these tables?

I think we need something like this: #362

@sfc-gh-okalaci sfc-gh-okalaci merged commit 3b192e4 into main May 20, 2026
64 checks passed
@sfc-gh-okalaci sfc-gh-okalaci deleted the okalaci/analyze_threshold_gate branch May 20, 2026 07:41
sfc-gh-okalaci added a commit that referenced this pull request May 25, 2026
The diff query in GetDataFileMetadataOperations joins lake_table.files,
data_file_partition_values, and data_file_column_stats; plan choice is
dominated by pg_class.reltuples. PG's defaults (scale_factor=0.10,
threshold=50) let reltuples drift enough on these high-churn catalogs
that the planner flips from hash join to nested loop.

Set scale_factor=0.05, threshold=500 on all three. Half PG's default
scale factor caps reltuples drift at ~5%, well inside planner
tolerance; lowering further is mostly ANALYZE thrash without a planner
benefit (diminishing returns). threshold=500 sits strictly below the
pg_lake_table.commit_time_analyze_threshold GUC (default 1000) from
#352, so autovacuum backstops the small-tx workload where the GUC
opts out.

Complements #352: the in-tx GUC handles large txs autovacuum cannot
reach; these storage params keep reltuples fresh between txs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants