From 792c2dd77e9826a2c954f1bb435c36fcab2d449c Mon Sep 17 00:00:00 2001 From: Anthonin Bonnefoy Date: Fri, 9 Feb 2024 09:04:04 +0100 Subject: 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 | 31 +++++++++++++++------------ src/backend/commands/analyze.c | 30 ++++++++++++++------------ 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, 33 insertions(+), 46 deletions(-) diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c index ba5b7083a3..41766ee97d 100644 --- a/src/backend/access/heap/vacuumlazy.c +++ b/src/backend/access/heap/vacuumlazy.c @@ -299,9 +299,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; @@ -593,18 +591,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) @@ -731,18 +729,23 @@ heap_vacuum_rel(Relation rel, VacuumParams *params, } if (secs_dur > 0 || usecs_dur > 0) { - read_rate = (double) BLCKSZ * PageMissOp / (1024 * 1024) / - (secs_dur + usecs_dur / 1000000.0); - write_rate = (double) BLCKSZ * PageDirtyOp / (1024 * 1024) / - (secs_dur + usecs_dur / 1000000.0); + read_rate = (double) BLCKSZ * (bufferusage.shared_blks_read + bufferusage.local_blks_read) / + (1024 * 1024) / (secs_dur + usecs_dur / 1000000.0); + write_rate = (double) BLCKSZ * (bufferusage.shared_blks_dirtied + bufferusage.local_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); + _("shared buffer usage: %lld hits, %lld misses, %lld dirtied\n"), + (long long) bufferusage.shared_blks_hit, + (long long) bufferusage.shared_blks_read, + (long long) bufferusage.shared_blks_dirtied); + appendStringInfo(&buf, + _("local buffer usage: %lld hits, %lld misses, %lld dirtied\n"), + (long long) bufferusage.local_blks_hit, + (long long) bufferusage.local_blks_read, + (long long) bufferusage.local_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 8a82af4a4c..60503db92e 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -303,9 +303,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; @@ -737,9 +736,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 @@ -765,10 +763,10 @@ do_analyze_rel(Relation onerel, VacuumParams *params, if (delay_in_ms > 0) { - read_rate = (double) BLCKSZ * AnalyzePageMiss / (1024 * 1024) / - (delay_in_ms / 1000.0); - write_rate = (double) BLCKSZ * AnalyzePageDirty / (1024 * 1024) / - (delay_in_ms / 1000.0); + read_rate = (double) BLCKSZ * (bufferusage.shared_blks_read + bufferusage.local_blks_read) / + (1024 * 1024) / (delay_in_ms / 1000.0); + write_rate = (double) BLCKSZ * (bufferusage.shared_blks_dirtied + bufferusage.local_blks_dirtied) / + (1024 * 1024) / (delay_in_ms / 1000.0); } /* @@ -791,10 +789,14 @@ 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); + appendStringInfo(&buf, _("shared buffer usage: %lld hits, %lld misses, %lld dirtied\n"), + (long long) bufferusage.shared_blks_hit, + (long long) bufferusage.shared_blks_read, + (long long) bufferusage.shared_blks_dirtied); + appendStringInfo(&buf, _("local buffer usage: %lld hits, %lld misses, %lld dirtied\n"), + (long long) bufferusage.local_blks_hit, + (long long) bufferusage.local_blks_read, + (long long) bufferusage.local_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 e63c86cae4..54daa5ad4b 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -604,9 +604,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 befda1c105..14d93eb081 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 f0f8d4259c..e8f290128c 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -1092,7 +1092,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) @@ -1198,7 +1197,6 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, TerminateBufferIO(bufHdr, false, BM_VALID, true); } - VacuumPageMiss++; if (VacuumCostActive) VacuumCostBalance += VacuumCostPageMiss; @@ -2228,7 +2226,6 @@ MarkBufferDirty(Buffer buffer) */ if (!(old_buf_state & BM_DIRTY)) { - VacuumPageDirty++; pgBufferUsage.shared_blks_dirtied++; if (VacuumCostActive) VacuumCostBalance += VacuumCostPageDirty; @@ -4746,7 +4743,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 3e38bb1311..d1b135aa91 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -151,10 +151,6 @@ 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 90f9b21b25..6d07005d39 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -283,10 +283,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-146)