From 87ff6cc66510233b1b86a250e8ac48e52b7d54c2 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 22 Jun 2020 16:36:14 -0500 Subject: [PATCH 3/4] Move error information into a separate struct.. ..to avoid copying large LVRelStats struct. Andres things this is a problem but Tom thinks it's a micro-optimization. See: b61d161c146328ae6ba9ed937862d66e5c8b035a This also moves relname and relnamespace (see ef75140fe). Discussion: https://www.postgresql.org/message-id/20200622200939.jkuniq3vtiumeszj%40alap3.anarazel.de --- src/backend/access/heap/vacuumlazy.c | 116 ++++++++++++++------------- 1 file changed, 62 insertions(+), 54 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 40173480b8..6bd2867660 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -288,10 +288,17 @@ typedef struct LVParallelState int nindexes_parallel_condcleanup; } LVParallelState; -typedef struct LVRelStats +typedef struct { char *relnamespace; char *relname; + char *indname; + BlockNumber blkno; /* used only for heap operations */ + VacErrPhase phase; +} LVSavedPosition; + +typedef struct LVRelStats +{ /* useindex = true means two-pass strategy; false means one-pass */ bool useindex; /* Overall statistics about rel */ @@ -313,10 +320,7 @@ typedef struct LVRelStats TransactionId latestRemovedXid; bool lock_waiter_detected; - /* Used for error callback */ - char *indname; - BlockNumber blkno; /* used only for heap operations */ - VacErrPhase phase; + LVSavedPosition errinfo; /* Used for error callback */ } LVRelStats; /* A few variables that don't seem worth passing around as parameters */ @@ -388,7 +392,7 @@ static void end_parallel_vacuum(Relation *Irel, IndexBulkDeleteResult **stats, static LVSharedIndStats *get_indstats(LVShared *lvshared, int n); static bool skip_parallel_vacuum_index(Relation indrel, LVShared *lvshared); static void vacuum_error_callback(void *arg); -static void update_vacuum_error_info(LVRelStats *errinfo, int phase, +static void update_vacuum_error_info(LVSavedPosition *errinfo, int phase, BlockNumber blkno, char *indname); @@ -475,10 +479,10 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats)); - vacrelstats->relnamespace = get_namespace_name(RelationGetNamespace(onerel)); - vacrelstats->relname = pstrdup(RelationGetRelationName(onerel)); - vacrelstats->indname = NULL; - vacrelstats->phase = VACUUM_ERRCB_PHASE_UNKNOWN; + vacrelstats->errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); + vacrelstats->errinfo.relname = pstrdup(RelationGetRelationName(onerel)); + vacrelstats->errinfo.indname = NULL; + vacrelstats->errinfo.phase = VACUUM_ERRCB_PHASE_UNKNOWN; vacrelstats->old_rel_pages = onerel->rd_rel->relpages; vacrelstats->old_live_tuples = onerel->rd_rel->reltuples; vacrelstats->num_index_scans = 0; @@ -502,7 +506,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, * information is restored at the end of those phases. */ errcallback.callback = vacuum_error_callback; - errcallback.arg = vacrelstats; + errcallback.arg = &vacrelstats->errinfo; errcallback.previous = error_context_stack; error_context_stack = &errcallback; @@ -538,7 +542,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, * which we add context information to errors, so we don't need to * revert to the previous phase. */ - update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_TRUNCATE, + update_vacuum_error_info(&vacrelstats->errinfo, + VACUUM_ERRCB_PHASE_TRUNCATE, vacrelstats->nonempty_pages, NULL); lazy_truncate_heap(onerel, vacrelstats); } @@ -650,8 +655,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params, } appendStringInfo(&buf, msgfmt, get_database_name(MyDatabaseId), - vacrelstats->relnamespace, - vacrelstats->relname, + vacrelstats->errinfo.relnamespace, + vacrelstats->errinfo.relname, vacrelstats->num_index_scans); appendStringInfo(&buf, _("pages: %u removed, %u remain, %u skipped due to pins, %u skipped frozen\n"), vacrelstats->pages_removed, @@ -786,13 +791,13 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, if (aggressive) ereport(elevel, (errmsg("aggressively vacuuming \"%s.%s\"", - vacrelstats->relnamespace, - vacrelstats->relname))); + vacrelstats->errinfo.relnamespace, + vacrelstats->errinfo.relname))); else ereport(elevel, (errmsg("vacuuming \"%s.%s\"", - vacrelstats->relnamespace, - vacrelstats->relname))); + vacrelstats->errinfo.relnamespace, + vacrelstats->errinfo.relname))); empty_pages = vacuumed_pages = 0; next_fsm_block_to_vacuum = (BlockNumber) 0; @@ -828,7 +833,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, if (params->nworkers > 0) ereport(WARNING, (errmsg("disabling parallel option of vacuum on \"%s\" --- cannot vacuum temporary tables in parallel", - vacrelstats->relname))); + vacrelstats->errinfo.relname))); } else lps = begin_parallel_vacuum(RelationGetRelid(onerel), Irel, @@ -948,7 +953,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno); - update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_SCAN_HEAP, + update_vacuum_error_info(&vacrelstats->errinfo, + VACUUM_ERRCB_PHASE_SCAN_HEAP, blkno, NULL); if (blkno == next_unskippable_block) @@ -1582,7 +1588,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, && VM_ALL_VISIBLE(onerel, blkno, &vmbuffer)) { elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u", - vacrelstats->relname, blkno); + vacrelstats->errinfo.relname, blkno); visibilitymap_clear(onerel, blkno, vmbuffer, VISIBILITYMAP_VALID_BITS); } @@ -1603,7 +1609,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, else if (PageIsAllVisible(page) && has_dead_tuples) { elog(WARNING, "page containing dead tuples is marked as all-visible in relation \"%s\" page %u", - vacrelstats->relname, blkno); + vacrelstats->errinfo.relname, blkno); PageClearAllVisible(page); MarkBufferDirty(buf); visibilitymap_clear(onerel, blkno, vmbuffer, @@ -1713,7 +1719,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, if (vacuumed_pages) ereport(elevel, (errmsg("\"%s\": removed %.0f row versions in %u pages", - vacrelstats->relname, + vacrelstats->errinfo.relname, tups_vacuumed, vacuumed_pages))); /* @@ -1742,7 +1748,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats, ereport(elevel, (errmsg("\"%s\": found %.0f removable, %.0f nonremovable row versions in %u out of %u pages", - vacrelstats->relname, + vacrelstats->errinfo.relname, tups_vacuumed, num_tuples, vacrelstats->scanned_pages, nblocks), errdetail_internal("%s", buf.data))); @@ -1820,15 +1826,16 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats) int npages; PGRUsage ru0; Buffer vmbuffer = InvalidBuffer; - LVRelStats olderrinfo; + LVSavedPosition olderrinfo; /* Report that we are now vacuuming the heap */ pgstat_progress_update_param(PROGRESS_VACUUM_PHASE, PROGRESS_VACUUM_PHASE_VACUUM_HEAP); /* Update error traceback information */ - olderrinfo = *vacrelstats; - update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP, + olderrinfo = vacrelstats->errinfo; + update_vacuum_error_info(&vacrelstats->errinfo, + VACUUM_ERRCB_PHASE_VACUUM_HEAP, InvalidBlockNumber, NULL); pg_rusage_init(&ru0); @@ -1845,7 +1852,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats) vacuum_delay_point(); tblk = ItemPointerGetBlockNumber(&vacrelstats->dead_tuples->itemptrs[tupindex]); - vacrelstats->blkno = tblk; + vacrelstats->errinfo.blkno = tblk; buf = ReadBufferExtended(onerel, MAIN_FORKNUM, tblk, RBM_NORMAL, vac_strategy); if (!ConditionalLockBufferForCleanup(buf)) @@ -1874,12 +1881,12 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats) ereport(elevel, (errmsg("\"%s\": removed %d row versions in %d pages", - vacrelstats->relname, + vacrelstats->errinfo.relname, tupindex, npages), errdetail_internal("%s", pg_rusage_show(&ru0)))); /* Revert to the previous phase information for error traceback */ - update_vacuum_error_info(vacrelstats, + update_vacuum_error_info(&vacrelstats->errinfo, olderrinfo.phase, olderrinfo.blkno, olderrinfo.indname); @@ -1905,13 +1912,14 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer, int uncnt = 0; TransactionId visibility_cutoff_xid; bool all_frozen; - LVRelStats olderrinfo; + LVSavedPosition olderrinfo; pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_VACUUMED, blkno); /* Update error traceback information */ - olderrinfo = *vacrelstats; - update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP, + olderrinfo = vacrelstats->errinfo; + update_vacuum_error_info(&vacrelstats->errinfo, + VACUUM_ERRCB_PHASE_VACUUM_HEAP, blkno, NULL); START_CRIT_SECTION(); @@ -1991,7 +1999,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer, } /* Revert to the previous phase information for error traceback */ - update_vacuum_error_info(vacrelstats, + update_vacuum_error_info(&vacrelstats->errinfo, olderrinfo.phase, olderrinfo.blkno, olderrinfo.indname); @@ -2404,7 +2412,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats, IndexVacuumInfo ivinfo; const char *msg; PGRUsage ru0; - LVRelStats olderrinfo; + LVSavedPosition olderrinfo; pg_rusage_init(&ru0); @@ -2417,8 +2425,8 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats, ivinfo.strategy = vac_strategy; /* Update error traceback information */ - olderrinfo = *vacrelstats; - update_vacuum_error_info(vacrelstats, + olderrinfo = vacrelstats->errinfo; + update_vacuum_error_info(&vacrelstats->errinfo, VACUUM_ERRCB_PHASE_VACUUM_INDEX, InvalidBlockNumber, RelationGetRelationName(indrel)); @@ -2434,12 +2442,12 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats, ereport(elevel, (errmsg(msg, - vacrelstats->indname, + vacrelstats->errinfo.indname, dead_tuples->num_tuples), errdetail_internal("%s", pg_rusage_show(&ru0)))); /* Revert to the previous phase information for error traceback */ - update_vacuum_error_info(vacrelstats, + update_vacuum_error_info(&vacrelstats->errinfo, olderrinfo.phase, olderrinfo.blkno, olderrinfo.indname); @@ -2459,7 +2467,7 @@ lazy_cleanup_index(Relation indrel, IndexVacuumInfo ivinfo; const char *msg; PGRUsage ru0; - LVRelStats olderrinfo; + LVSavedPosition olderrinfo; pg_rusage_init(&ru0); @@ -2473,8 +2481,8 @@ lazy_cleanup_index(Relation indrel, ivinfo.strategy = vac_strategy; /* Update error traceback information */ - olderrinfo = *vacrelstats; - update_vacuum_error_info(vacrelstats, + olderrinfo = vacrelstats->errinfo; + update_vacuum_error_info(&vacrelstats->errinfo, VACUUM_ERRCB_PHASE_INDEX_CLEANUP, InvalidBlockNumber, RelationGetRelationName(indrel)); @@ -2482,7 +2490,7 @@ lazy_cleanup_index(Relation indrel, *stats = index_vacuum_cleanup(&ivinfo, *stats); /* Revert back to the old phase information for error traceback */ - update_vacuum_error_info(vacrelstats, + update_vacuum_error_info(&vacrelstats->errinfo, olderrinfo.phase, olderrinfo.blkno, olderrinfo.indname); @@ -2597,7 +2605,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats) vacrelstats->lock_waiter_detected = true; ereport(elevel, (errmsg("\"%s\": stopping truncate due to conflicting lock request", - vacrelstats->relname))); + vacrelstats->errinfo.relname))); return; } @@ -2630,7 +2638,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats) * were vacuuming. */ new_rel_pages = count_nondeletable_pages(onerel, vacrelstats); - vacrelstats->blkno = new_rel_pages; + vacrelstats->errinfo.blkno = new_rel_pages; if (new_rel_pages >= old_rel_pages) { @@ -2663,7 +2671,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats) ereport(elevel, (errmsg("\"%s\": truncated %u to %u pages", - vacrelstats->relname, + vacrelstats->errinfo.relname, old_rel_pages, new_rel_pages), errdetail_internal("%s", pg_rusage_show(&ru0)))); @@ -2728,7 +2736,7 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats) { ereport(elevel, (errmsg("\"%s\": suspending truncate due to conflicting lock request", - vacrelstats->relname))); + vacrelstats->errinfo.relname))); vacrelstats->lock_waiter_detected = true; return blkno; @@ -3521,14 +3529,14 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) * Initialize vacrelstats for use as error callback arg by parallel * worker. */ - vacrelstats.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); - vacrelstats.relname = pstrdup(RelationGetRelationName(onerel)); - vacrelstats.indname = NULL; - vacrelstats.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */ + vacrelstats.errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); + vacrelstats.errinfo.relname = pstrdup(RelationGetRelationName(onerel)); + vacrelstats.errinfo.indname = NULL; + vacrelstats.errinfo.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */ /* Setup error traceback support for ereport() */ errcallback.callback = vacuum_error_callback; - errcallback.arg = &vacrelstats; + errcallback.arg = &vacrelstats.errinfo; errcallback.previous = error_context_stack; error_context_stack = &errcallback; @@ -3559,7 +3567,7 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) static void vacuum_error_callback(void *arg) { - LVRelStats *errinfo = arg; + LVSavedPosition *errinfo = arg; switch (errinfo->phase) { @@ -3600,7 +3608,7 @@ vacuum_error_callback(void *arg) /* Update vacuum error callback for the current phase, block, and index. */ static void -update_vacuum_error_info(LVRelStats *errinfo, int phase, BlockNumber blkno, +update_vacuum_error_info(LVSavedPosition *errinfo, int phase, BlockNumber blkno, char *indname) { errinfo->blkno = blkno; -- 2.17.0