Skip to content

Commit 2252fcd

Browse files
michaelpqmichaelpq
committed
Rationalize handling of VacuumParams
This commit refactors the vacuum routines that rely on VacuumParams, adding const markers where necessary to force a new policy in the code. This structure should not use a pointer as it may be used across multiple relations, and its contents should never be updated. vacuum_rel() stands as an exception as it touches the "index_cleanup" and "truncate" options. VacuumParams has been introduced in 0d83138, and 661643d has fixed a bug impacting VACUUM operating on multiple relations. The changes done in tableam.h break ABI compatibility, so this commit can only happen on HEAD. Author: Shihao Zhong <zhong950419@gmail.com> Co-authored-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Junwang Zhao <zhjwpku@gmail.com> Discussion: https://postgr.es/m/CAGRkXqTo+aK=GTy5pSc-9cy8H2F2TJvcrZ-zXEiNJj93np1UUw@mail.gmail.com
1 parent 5ba00e1 commit 2252fcd

File tree

8 files changed

+98
-110
lines changed

8 files changed

+98
-110
lines changed

‎src/backend/access/heap/vacuumlazy.c

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ typedef struct LVSavedErrInfo
423423
/* non-export function prototypes */
424424
static void lazy_scan_heap(LVRelState *vacrel);
425425
static void heap_vacuum_eager_scan_setup(LVRelState *vacrel,
426-
VacuumParams *params);
426+
const VacuumParams params);
427427
static BlockNumber heap_vac_scan_next_block(ReadStream *stream,
428428
void *callback_private_data,
429429
void *per_buffer_data);
@@ -485,7 +485,7 @@ static void restore_vacuum_error_info(LVRelState *vacrel,
485485
* vacuum options or for relfrozenxid/relminmxid advancement.
486486
*/
487487
static void
488-
heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params)
488+
heap_vacuum_eager_scan_setup(LVRelState *vacrel, const VacuumParams params)
489489
{
490490
uint32 randseed;
491491
BlockNumber allvisible;
@@ -504,7 +504,7 @@ heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params)
504504
vacrel->eager_scan_remaining_successes = 0;
505505

506506
/* If eager scanning is explicitly disabled, just return. */
507-
if (params->max_eager_freeze_failure_rate == 0)
507+
if (params.max_eager_freeze_failure_rate == 0)
508508
return;
509509

510510
/*
@@ -581,11 +581,11 @@ heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params)
581581

582582
vacrel->next_eager_scan_region_start = randseed % EAGER_SCAN_REGION_SIZE;
583583

584-
Assert(params->max_eager_freeze_failure_rate > 0 &&
585-
params->max_eager_freeze_failure_rate <= 1);
584+
Assert(params.max_eager_freeze_failure_rate > 0 &&
585+
params.max_eager_freeze_failure_rate <= 1);
586586

587587
vacrel->eager_scan_max_fails_per_region =
588-
params->max_eager_freeze_failure_rate *
588+
params.max_eager_freeze_failure_rate *
589589
EAGER_SCAN_REGION_SIZE;
590590

591591
/*
@@ -612,7 +612,7 @@ heap_vacuum_eager_scan_setup(LVRelState *vacrel, VacuumParams *params)
612612
* and locked the relation.
613613
*/
614614
void
615-
heap_vacuum_rel(Relation rel, VacuumParams *params,
615+
heap_vacuum_rel(Relation rel, const VacuumParams params,
616616
BufferAccessStrategy bstrategy)
617617
{
618618
LVRelState *vacrel;
@@ -634,9 +634,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
634634
ErrorContextCallback errcallback;
635635
char **indnames = NULL;
636636

637-
verbose = (params->options & VACOPT_VERBOSE) != 0;
637+
verbose = (params.options & VACOPT_VERBOSE) != 0;
638638
instrument = (verbose || (AmAutoVacuumWorkerProcess() &&
639-
params->log_min_duration >= 0));
639+
params.log_min_duration >= 0));
640640
if (instrument)
641641
{
642642
pg_rusage_init(&ru0);
@@ -699,9 +699,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
699699
* The truncate param allows user to avoid attempting relation truncation,
700700
* though it can't force truncation to happen.
701701
*/
702-
Assert(params->index_cleanup != VACOPTVALUE_UNSPECIFIED);
703-
Assert(params->truncate != VACOPTVALUE_UNSPECIFIED &&
704-
params->truncate != VACOPTVALUE_AUTO);
702+
Assert(params.index_cleanup != VACOPTVALUE_UNSPECIFIED);
703+
Assert(params.truncate != VACOPTVALUE_UNSPECIFIED &&
704+
params.truncate != VACOPTVALUE_AUTO);
705705

706706
/*
707707
* While VacuumFailSafeActive is reset to false before calling this, we
@@ -711,22 +711,22 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
711711
vacrel->consider_bypass_optimization = true;
712712
vacrel->do_index_vacuuming = true;
713713
vacrel->do_index_cleanup = true;
714-
vacrel->do_rel_truncate = (params->truncate != VACOPTVALUE_DISABLED);
715-
if (params->index_cleanup == VACOPTVALUE_DISABLED)
714+
vacrel->do_rel_truncate = (params.truncate != VACOPTVALUE_DISABLED);
715+
if (params.index_cleanup == VACOPTVALUE_DISABLED)
716716
{
717717
/* Force disable index vacuuming up-front */
718718
vacrel->do_index_vacuuming = false;
719719
vacrel->do_index_cleanup = false;
720720
}
721-
else if (params->index_cleanup == VACOPTVALUE_ENABLED)
721+
else if (params.index_cleanup == VACOPTVALUE_ENABLED)
722722
{
723723
/* Force index vacuuming. Note that failsafe can still bypass. */
724724
vacrel->consider_bypass_optimization = false;
725725
}
726726
else
727727
{
728728
/* Default/auto, make all decisions dynamically */
729-
Assert(params->index_cleanup == VACOPTVALUE_AUTO);
729+
Assert(params.index_cleanup == VACOPTVALUE_AUTO);
730730
}
731731

732732
/* Initialize page counters explicitly (be tidy) */
@@ -789,7 +789,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
789789
*/
790790
vacrel->skippedallvis = false;
791791
skipwithvm = true;
792-
if (params->options & VACOPT_DISABLE_PAGE_SKIPPING)
792+
if (params.options & VACOPT_DISABLE_PAGE_SKIPPING)
793793
{
794794
/*
795795
* Force aggressive mode, and disable skipping blocks using the
@@ -830,7 +830,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
830830
* is already dangerously old.)
831831
*/
832832
lazy_check_wraparound_failsafe(vacrel);
833-
dead_items_alloc(vacrel, params->nworkers);
833+
dead_items_alloc(vacrel, params.nworkers);
834834

835835
/*
836836
* Call lazy_scan_heap to perform all required heap pruning, index
@@ -947,9 +947,9 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
947947
{
948948
TimestampTz endtime = GetCurrentTimestamp();
949949

950-
if (verbose || params->log_min_duration == 0 ||
950+
if (verbose || params.log_min_duration == 0 ||
951951
TimestampDifferenceExceeds(starttime, endtime,
952-
params->log_min_duration))
952+
params.log_min_duration))
953953
{
954954
long secs_dur;
955955
int usecs_dur;
@@ -984,10 +984,10 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
984984
* Aggressiveness already reported earlier, in dedicated
985985
* VACUUM VERBOSE ereport
986986
*/
987-
Assert(!params->is_wraparound);
987+
Assert(!params.is_wraparound);
988988
msgfmt = _("finished vacuuming \"%s.%s.%s\": index scans: %d\n");
989989
}
990-
else if (params->is_wraparound)
990+
else if (params.is_wraparound)
991991
{
992992
/*
993993
* While it's possible for a VACUUM to be both is_wraparound

‎src/backend/commands/analyze.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ static BufferAccessStrategy vac_strategy;
7676

7777

7878
static void do_analyze_rel(Relation onerel,
79-
VacuumParams *params, List *va_cols,
79+
const VacuumParams params, List *va_cols,
8080
AcquireSampleRowsFunc acquirefunc, BlockNumber relpages,
8181
bool inh, bool in_outer_xact, int elevel);
8282
static void compute_index_stats(Relation onerel, double totalrows,
@@ -107,7 +107,7 @@ static Datum ind_fetch_func(VacAttrStatsP stats, int rownum, bool *isNull);
107107
*/
108108
void
109109
analyze_rel(Oid relid, RangeVar *relation,
110-
VacuumParams *params, List *va_cols, bool in_outer_xact,
110+
const VacuumParams params, List *va_cols, bool in_outer_xact,
111111
BufferAccessStrategy bstrategy)
112112
{
113113
Relation onerel;
@@ -116,7 +116,7 @@ analyze_rel(Oid relid, RangeVar *relation,
116116
BlockNumber relpages = 0;
117117

118118
/* Select logging level */
119-
if (params->options & VACOPT_VERBOSE)
119+
if (params.options & VACOPT_VERBOSE)
120120
elevel = INFO;
121121
else
122122
elevel = DEBUG2;
@@ -138,8 +138,8 @@ analyze_rel(Oid relid, RangeVar *relation,
138138
*
139139
* Make sure to generate only logs for ANALYZE in this case.
140140
*/
141-
onerel = vacuum_open_relation(relid, relation, params->options & ~(VACOPT_VACUUM),
142-
params->log_min_duration >= 0,
141+
onerel = vacuum_open_relation(relid, relation, params.options & ~(VACOPT_VACUUM),
142+
params.log_min_duration >= 0,
143143
ShareUpdateExclusiveLock);
144144

145145
/* leave if relation could not be opened or locked */
@@ -155,7 +155,7 @@ analyze_rel(Oid relid, RangeVar *relation,
155155
*/
156156
if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
157157
onerel->rd_rel,
158-
params->options & ~VACOPT_VACUUM))
158+
params.options & ~VACOPT_VACUUM))
159159
{
160160
relation_close(onerel, ShareUpdateExclusiveLock);
161161
return;
@@ -227,7 +227,7 @@ analyze_rel(Oid relid, RangeVar *relation,
227227
else
228228
{
229229
/* No need for a WARNING if we already complained during VACUUM */
230-
if (!(params->options & VACOPT_VACUUM))
230+
if (!(params.options & VACOPT_VACUUM))
231231
ereport(WARNING,
232232
(errmsg("skipping \"%s\" --- cannot analyze non-tables or special system tables",
233233
RelationGetRelationName(onerel))));
@@ -275,7 +275,7 @@ analyze_rel(Oid relid, RangeVar *relation,
275275
* appropriate acquirefunc for each child table.
276276
*/
277277
static void
278-
do_analyze_rel(Relation onerel, VacuumParams *params,
278+
do_analyze_rel(Relation onerel, const VacuumParams params,
279279
List *va_cols, AcquireSampleRowsFunc acquirefunc,
280280
BlockNumber relpages, bool inh, bool in_outer_xact,
281281
int elevel)
@@ -309,9 +309,9 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
309309
PgStat_Counter startreadtime = 0;
310310
PgStat_Counter startwritetime = 0;
311311

312-
verbose = (params->options & VACOPT_VERBOSE) != 0;
312+
verbose = (params.options & VACOPT_VERBOSE) != 0;
313313
instrument = (verbose || (AmAutoVacuumWorkerProcess() &&
314-
params->log_min_duration >= 0));
314+
params.log_min_duration >= 0));
315315
if (inh)
316316
ereport(elevel,
317317
(errmsg("analyzing \"%s.%s\" inheritance tree",
@@ -706,7 +706,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
706706
* amvacuumcleanup() when called in ANALYZE-only mode. The only exception
707707
* among core index AMs is GIN/ginvacuumcleanup().
708708
*/
709-
if (!(params->options & VACOPT_VACUUM))
709+
if (!(params.options & VACOPT_VACUUM))
710710
{
711711
for (ind = 0; ind < nindexes; ind++)
712712
{
@@ -736,9 +736,9 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
736736
{
737737
TimestampTz endtime = GetCurrentTimestamp();
738738

739-
if (verbose || params->log_min_duration == 0 ||
739+
if (verbose || params.log_min_duration == 0 ||
740740
TimestampDifferenceExceeds(starttime, endtime,
741-
params->log_min_duration))
741+
params.log_min_duration))
742742
{
743743
long delay_in_ms;
744744
WalUsage walusage;

‎src/backend/commands/cluster.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -917,7 +917,7 @@ copy_table_data(Relation NewHeap, Relation OldHeap, Relation OldIndex, bool verb
917917
* not to be aggressive about this.
918918
*/
919919
memset(&params, 0, sizeof(VacuumParams));
920-
vacuum_get_cutoffs(OldHeap, &params, &cutoffs);
920+
vacuum_get_cutoffs(OldHeap, params, &cutoffs);
921921

922922
/*
923923
* FreezeXid will become the table's new relfrozenxid, and that mustn't go

0 commit comments

Comments
 (0)