From 5ab4695bc59ee5e79e937590bcd58ff82da80f92 Mon Sep 17 00:00:00 2001 From: Anthonin Bonnefoy Date: Fri, 9 Feb 2024 09:04:04 +0100 Subject: [PATCH v01] Fix parallel vacuum buffer usage reporting Vacuum uses dedicated VacuumPage{Hit,Miss,Dirty} globals to track buffer usage. However, those values are not collected at the end of parallel vacuum workers, leading to incorrect buffer count. This patch fix this issue by removing the dedicated page tracking for vacuum and using pgBufferUsage instead. Buffer tracking is already done in pgBufferUsage shared_blks_{hit,read,dirtied}, making the vacuum specific variables redundant. Moreover, pgBufferUsage is already collected at the end of parallel workers, leading to a correct result of buffer used. --- src/backend/access/heap/vacuumlazy.c | 20 +++++++++----------- src/backend/commands/analyze.c | 20 +++++++++----------- src/backend/commands/vacuum.c | 3 --- src/backend/commands/vacuumparallel.c | 3 --- src/backend/storage/buffer/bufmgr.c | 4 ---- src/backend/utils/init/globals.c | 4 ---- src/include/miscadmin.h | 4 ---- 7 files changed, 18 insertions(+), 40 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index fa56480808..43924da32c 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -300,9 +300,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, PgStat_Counter startreadtime = 0, startwritetime = 0; WalUsage startwalusage = pgWalUsage; - int64 StartPageHit = VacuumPageHit, - StartPageMiss = VacuumPageMiss, - StartPageDirty = VacuumPageDirty; + BufferUsage startbufferusage = pgBufferUsage; ErrorContextCallback errcallback; char **indnames = NULL; @@ -594,18 +592,18 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, long secs_dur; int usecs_dur; WalUsage walusage; + BufferUsage bufferusage; StringInfoData buf; char *msgfmt; int32 diff; - int64 PageHitOp = VacuumPageHit - StartPageHit, - PageMissOp = VacuumPageMiss - StartPageMiss, - PageDirtyOp = VacuumPageDirty - StartPageDirty; double read_rate = 0, write_rate = 0; TimestampDifference(starttime, endtime, &secs_dur, &usecs_dur); memset(&walusage, 0, sizeof(WalUsage)); WalUsageAccumDiff(&walusage, &pgWalUsage, &startwalusage); + memset(&bufferusage, 0, sizeof(BufferUsage)); + BufferUsageAccumDiff(&bufferusage, &pgBufferUsage, &startbufferusage); initStringInfo(&buf); if (verbose) @@ -732,18 +730,18 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, } if (secs_dur > 0 || usecs_dur > 0) { - read_rate = (double) BLCKSZ * PageMissOp / (1024 * 1024) / + read_rate = (double) BLCKSZ * bufferusage.shared_blks_read / (1024 * 1024) / (secs_dur + usecs_dur / 1000000.0); - write_rate = (double) BLCKSZ * PageDirtyOp / (1024 * 1024) / + write_rate = (double) BLCKSZ * bufferusage.shared_blks_dirtied / (1024 * 1024) / (secs_dur + usecs_dur / 1000000.0); } appendStringInfo(&buf, _("avg read rate: %.3f MB/s, avg write rate: %.3f MB/s\n"), read_rate, write_rate); appendStringInfo(&buf, _("buffer usage: %lld hits, %lld misses, %lld dirtied\n"), - (long long) PageHitOp, - (long long) PageMissOp, - (long long) PageDirtyOp); + (long long) bufferusage.shared_blks_hit, + (long long) bufferusage.shared_blks_read, + (long long) bufferusage.shared_blks_dirtied); appendStringInfo(&buf, _("WAL usage: %lld records, %lld full page images, %llu bytes\n"), (long long) walusage.wal_records, diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index a03495d6c9..17d713a7a7 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -314,9 +314,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params, Oid save_userid; int save_sec_context; int save_nestlevel; - int64 AnalyzePageHit = VacuumPageHit; - int64 AnalyzePageMiss = VacuumPageMiss; - int64 AnalyzePageDirty = VacuumPageDirty; + BufferUsage startbufferusage = pgBufferUsage; + BufferUsage bufferusage; PgStat_Counter startreadtime = 0; PgStat_Counter startwritetime = 0; @@ -747,9 +746,8 @@ do_analyze_rel(Relation onerel, VacuumParams *params, * happened as part of the analyze by subtracting out the * pre-analyze values which we saved above. */ - AnalyzePageHit = VacuumPageHit - AnalyzePageHit; - AnalyzePageMiss = VacuumPageMiss - AnalyzePageMiss; - AnalyzePageDirty = VacuumPageDirty - AnalyzePageDirty; + memset(&bufferusage, 0, sizeof(BufferUsage)); + BufferUsageAccumDiff(&bufferusage, &pgBufferUsage, &startbufferusage); /* * We do not expect an analyze to take > 25 days and it simplifies @@ -775,9 +773,9 @@ do_analyze_rel(Relation onerel, VacuumParams *params, if (delay_in_ms > 0) { - read_rate = (double) BLCKSZ * AnalyzePageMiss / (1024 * 1024) / + read_rate = (double) BLCKSZ * bufferusage.shared_blks_read / (1024 * 1024) / (delay_in_ms / 1000.0); - write_rate = (double) BLCKSZ * AnalyzePageDirty / (1024 * 1024) / + write_rate = (double) BLCKSZ * bufferusage.shared_blks_dirtied / (1024 * 1024) / (delay_in_ms / 1000.0); } @@ -802,9 +800,9 @@ do_analyze_rel(Relation onerel, VacuumParams *params, appendStringInfo(&buf, _("avg read rate: %.3f MB/s, avg write rate: %.3f MB/s\n"), read_rate, write_rate); appendStringInfo(&buf, _("buffer usage: %lld hits, %lld misses, %lld dirtied\n"), - (long long) AnalyzePageHit, - (long long) AnalyzePageMiss, - (long long) AnalyzePageDirty); + (long long) bufferusage.shared_blks_hit, + (long long) bufferusage.shared_blks_read, + (long long) bufferusage.shared_blks_dirtied); appendStringInfo(&buf, _("system usage: %s"), pg_rusage_show(&ru0)); ereport(LOG, diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 64da848627..70b592d70b 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -603,9 +603,6 @@ vacuum(List *relations, VacuumParams *params, BufferAccessStrategy bstrategy, VacuumFailsafeActive = false; VacuumUpdateCosts(); VacuumCostBalance = 0; - VacuumPageHit = 0; - VacuumPageMiss = 0; - VacuumPageDirty = 0; VacuumCostBalanceLocal = 0; VacuumSharedCostBalance = NULL; VacuumActiveNWorkers = NULL; diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c index e087dfd72e..a3165c8f3b 100644 --- a/src/backend/commands/vacuumparallel.c +++ b/src/backend/commands/vacuumparallel.c @@ -1013,9 +1013,6 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc) /* Set cost-based vacuum delay */ VacuumUpdateCosts(); VacuumCostBalance = 0; - VacuumPageHit = 0; - VacuumPageMiss = 0; - VacuumPageDirty = 0; VacuumCostBalanceLocal = 0; VacuumSharedCostBalance = &(shared->cost_balance); VacuumActiveNWorkers = &(shared->active_nworkers); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index eb1ec3b86d..46ce8a2343 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -1093,7 +1093,6 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, { /* Just need to update stats before we exit */ *hit = true; - VacuumPageHit++; pgstat_count_io_op(io_object, io_context, IOOP_HIT); if (VacuumCostActive) @@ -1199,7 +1198,6 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, TerminateBufferIO(bufHdr, false, BM_VALID, true); } - VacuumPageMiss++; if (VacuumCostActive) VacuumCostBalance += VacuumCostPageMiss; @@ -2229,7 +2227,6 @@ MarkBufferDirty(Buffer buffer) */ if (!(old_buf_state & BM_DIRTY)) { - VacuumPageDirty++; pgBufferUsage.shared_blks_dirtied++; if (VacuumCostActive) VacuumCostBalance += VacuumCostPageDirty; @@ -4747,7 +4744,6 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std) if (dirtied) { - VacuumPageDirty++; pgBufferUsage.shared_blks_dirtied++; if (VacuumCostActive) VacuumCostBalance += VacuumCostPageDirty; diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 88b03e8fa3..2732baba4b 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -150,9 +150,5 @@ int VacuumCostPageDirty = 20; int VacuumCostLimit = 200; double VacuumCostDelay = 0; -int64 VacuumPageHit = 0; -int64 VacuumPageMiss = 0; -int64 VacuumPageDirty = 0; - int VacuumCostBalance = 0; /* working state for vacuum */ bool VacuumCostActive = false; diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 0b01c1f093..841b445136 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -280,10 +280,6 @@ extern PGDLLIMPORT int VacuumCostPageDirty; extern PGDLLIMPORT int VacuumCostLimit; extern PGDLLIMPORT double VacuumCostDelay; -extern PGDLLIMPORT int64 VacuumPageHit; -extern PGDLLIMPORT int64 VacuumPageMiss; -extern PGDLLIMPORT int64 VacuumPageDirty; - extern PGDLLIMPORT int VacuumCostBalance; extern PGDLLIMPORT bool VacuumCostActive; -- 2.39.3 (Apple Git-145)