From 78d378fd22f74828981692e5e253214bccef4311 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 22 Jun 2020 16:36:14 -0500 Subject: [PATCH v2 4/5] 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 | 128 +++++++++++++++------------ 1 file changed, 72 insertions(+), 56 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index 2393269565..9eb4bc66ae 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); @@ -475,9 +479,9 @@ 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->phase = VACUUM_ERRCB_PHASE_UNKNOWN; + vacrelstats->errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); + vacrelstats->errinfo.relname = pstrdup(RelationGetRelationName(onerel)); + 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; @@ -501,7 +505,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; @@ -537,7 +541,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); lazy_truncate_heap(onerel, vacrelstats); } @@ -649,8 +654,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, @@ -785,13 +790,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; @@ -827,7 +832,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, @@ -947,7 +952,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); if (blkno == next_unskippable_block) @@ -1581,7 +1587,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); } @@ -1602,7 +1608,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, @@ -1712,7 +1718,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))); /* @@ -1741,7 +1747,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))); @@ -1819,15 +1825,16 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats) int npages; PGRUsage ru0; Buffer vmbuffer = InvalidBuffer; - int oldphase = vacrelstats->phase, - oldblkno = vacrelstats->blkno; + 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 */ - 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); pg_rusage_init(&ru0); @@ -1844,7 +1851,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)) @@ -1873,12 +1880,14 @@ 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, oldphase, oldblkno); + update_vacuum_error_info(&vacrelstats->errinfo, + olderrinfo.phase, + olderrinfo.blkno); } /* @@ -1901,13 +1910,14 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer, int uncnt = 0; TransactionId visibility_cutoff_xid; bool all_frozen; - int oldphase = vacrelstats->phase, - oldblkno = vacrelstats->blkno; + LVSavedPosition olderrinfo; pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_VACUUMED, blkno); /* Update error traceback information */ - 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); START_CRIT_SECTION(); @@ -1987,7 +1997,9 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer, } /* Revert to the previous phase information for error traceback */ - update_vacuum_error_info(vacrelstats, oldphase, oldblkno); + update_vacuum_error_info(&vacrelstats->errinfo, + olderrinfo.phase, + olderrinfo.blkno); return tupindex; } @@ -2397,8 +2409,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats, IndexVacuumInfo ivinfo; const char *msg; PGRUsage ru0; - int oldphase = vacrelstats->phase, - oldblkno = vacrelstats->blkno; + LVSavedPosition olderrinfo; pg_rusage_init(&ru0); @@ -2411,9 +2422,10 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats, ivinfo.strategy = vac_strategy; /* Update error traceback information */ + olderrinfo = vacrelstats->errinfo; /* The index name is also saved during this phase */ - vacrelstats->indname = pstrdup(RelationGetRelationName(indrel)); - update_vacuum_error_info(vacrelstats, + vacrelstats->errinfo.indname = pstrdup(RelationGetRelationName(indrel)); + update_vacuum_error_info(&vacrelstats->errinfo, VACUUM_ERRCB_PHASE_VACUUM_INDEX, InvalidBlockNumber); @@ -2428,13 +2440,15 @@ 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, oldphase, oldblkno); - pfree(vacrelstats->indname); + update_vacuum_error_info(&vacrelstats->errinfo, + olderrinfo.phase, + olderrinfo.blkno); + pfree(vacrelstats->errinfo.indname); } /* @@ -2451,8 +2465,7 @@ lazy_cleanup_index(Relation indrel, IndexVacuumInfo ivinfo; const char *msg; PGRUsage ru0; - int oldphase = vacrelstats->phase, - oldblkno = vacrelstats->blkno; + LVSavedPosition olderrinfo; pg_rusage_init(&ru0); @@ -2466,17 +2479,20 @@ lazy_cleanup_index(Relation indrel, ivinfo.strategy = vac_strategy; /* Update error traceback information */ + olderrinfo = vacrelstats->errinfo; /* The index name is also saved during this phase */ - vacrelstats->indname = pstrdup(RelationGetRelationName(indrel)); - update_vacuum_error_info(vacrelstats, + vacrelstats->errinfo.indname = pstrdup(RelationGetRelationName(indrel)); + update_vacuum_error_info(&vacrelstats->errinfo, VACUUM_ERRCB_PHASE_INDEX_CLEANUP, InvalidBlockNumber); *stats = index_vacuum_cleanup(&ivinfo, *stats); /* Revert back to the old phase information for error traceback */ - update_vacuum_error_info(vacrelstats, oldphase, oldblkno); - pfree(vacrelstats->indname); + update_vacuum_error_info(&vacrelstats->errinfo, + olderrinfo.phase, + olderrinfo.blkno); + pfree(vacrelstats->errinfo.indname); if (!(*stats)) return; @@ -2589,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; } @@ -2622,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) { @@ -2655,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)))); @@ -2720,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; @@ -3513,13 +3529,13 @@ 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.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */ + vacrelstats.errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel)); + vacrelstats.errinfo.relname = pstrdup(RelationGetRelationName(onerel)); + 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; @@ -3550,7 +3566,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) { @@ -3591,7 +3607,7 @@ vacuum_error_callback(void *arg) /* Update vacuum error callback for the current phase and block. */ static void -update_vacuum_error_info(LVRelStats *errinfo, int phase, BlockNumber blkno) +update_vacuum_error_info(LVSavedPosition *errinfo, int phase, BlockNumber blkno) { errinfo->blkno = blkno; errinfo->phase = phase; -- 2.17.0