Remove dependency on VacuumPage(Hit/Miss/Dirty) counters in do_analyze_rel
As part of commit 5cd72cc0c5017a9d4de8b5d465a75946da5abd1d, the
dependency on global counters such as VacuumPage(Hit/Miss/Dirty) was
removed from the vacuum. However, do_analyze_rel() was still using
these counters, necessitating the tracking of global counters
alongside BufferUsage counters.
The attached patch addresses the issue by eliminating the need to
track VacuumPage(Hit/Miss/Dirty) counters in do_analyze_rel(), making
the global counters obsolete. This simplifies the code and improves
consistency.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
Attachments:
v1-0001-Remove-duplicate-tracking-of-the-page-stats-durin.patchapplication/octet-stream; name=v1-0001-Remove-duplicate-tracking-of-the-page-stats-durin.patchDownload
From f88f0a85c9e073b719c4b4f0ebbc496184e65950 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilip.kumar@enterprisedb.com>
Date: Thu, 6 Jun 2024 14:29:11 +0530
Subject: [PATCH v1] Remove duplicate tracking of the page stats during analyze
As part of commit 5cd72cc0c5017a9d4de8b5d465a75946da5abd1d, the dependency
on global counters such as VacuumPage(Hit/Miss/Dirty) was removed from the vacuum.
However, do_analyze_rel() was still using these counters, necessitating the
tracking of global counters alongside BufferUsage counters.
This commit addresses the issue by eliminating the need to track
VacuumPage(Hit/Miss/Dirty) counters in do_analyze_rel(), making the global
counters obsolete. This simplifies the code and improves consistency.
Dilip Kumar
---
src/backend/commands/analyze.c | 26 ++++++++++++--------------
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 ----
6 files changed, 12 insertions(+), 32 deletions(-)
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 7d2cd24997..8e6614eb32 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -303,9 +303,7 @@ 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;
PgStat_Counter startreadtime = 0;
PgStat_Counter startwritetime = 0;
@@ -730,6 +728,7 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
long delay_in_ms;
double read_rate = 0;
double write_rate = 0;
+ BufferUsage bufferusage;
StringInfoData buf;
/*
@@ -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,10 @@ 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, _("buffer usage_new: %lld hits, %lld misses, %lld dirtied\n"),
+ (long long) (bufferusage.shared_blks_hit + bufferusage.local_blks_hit),
+ (long long) (bufferusage.shared_blks_read + bufferusage.local_blks_read),
+ (long long) (bufferusage.shared_blks_dirtied + 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 48f8eab202..7d8e9d2045 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 f26070bff2..22c057fe61 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -1043,9 +1043,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 49637284f9..996ee6fdc8 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1127,7 +1127,6 @@ PinBufferForBlock(Relation rel,
}
if (*foundPtr)
{
- VacuumPageHit++;
pgstat_count_io_op(io_object, io_context, IOOP_HIT);
if (VacuumCostActive)
VacuumCostBalance += VacuumCostPageHit;
@@ -1519,7 +1518,6 @@ WaitReadBuffers(ReadBuffersOperation *operation)
false);
}
- VacuumPageMiss += io_buffers_len;
if (VacuumCostActive)
VacuumCostBalance += VacuumCostPageMiss * io_buffers_len;
}
@@ -2513,7 +2511,6 @@ MarkBufferDirty(Buffer buffer)
*/
if (!(old_buf_state & BM_DIRTY))
{
- VacuumPageDirty++;
pgBufferUsage.shared_blks_dirtied++;
if (VacuumCostActive)
VacuumCostBalance += VacuumCostPageDirty;
@@ -5036,7 +5033,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 cc61937eef..ba7f45e8a2 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.2 (Apple Git-143)
Hi,
I sent a similar patch for this in
/messages/by-id/CAO6_Xqr__kTTCLkftqS0qSCm-J7_xbRG3Ge2rWhucxQJMJhcRA@mail.gmail.com
Regards,
Anthonin
On Thu, Jun 6, 2024 at 11:10 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
Show quoted text
As part of commit 5cd72cc0c5017a9d4de8b5d465a75946da5abd1d, the
dependency on global counters such as VacuumPage(Hit/Miss/Dirty) was
removed from the vacuum. However, do_analyze_rel() was still using
these counters, necessitating the tracking of global counters
alongside BufferUsage counters.The attached patch addresses the issue by eliminating the need to
track VacuumPage(Hit/Miss/Dirty) counters in do_analyze_rel(), making
the global counters obsolete. This simplifies the code and improves
consistency.--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com
On Thu, Jun 6, 2024 at 3:23 PM Anthonin Bonnefoy
<anthonin.bonnefoy@datadoghq.com> wrote:
Hi,
I sent a similar patch for this in /messages/by-id/CAO6_Xqr__kTTCLkftqS0qSCm-J7_xbRG3Ge2rWhucxQJMJhcRA@mail.gmail.com
Okay, I see, In that case, we can just discard mine, thanks for notifying me.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com