vacuumdb --analyze-only does not need to issue VACUUM (ONLY_DATABASE_STATS) ?

Started by Fujii Masao5 months ago6 messages
#1Fujii Masao
masao.fujii@gmail.com
1 attachment(s)

Hi,

I noticed that vacuumdb --analyze-only currently issues
VACUUM (ONLY_DATABASE_STATS). Is this actually needed?
BTW, vacuumdb does not run this command when --analyze-in-stages
is specified.

The attached patch modifies vacuumdb to skip
VACUUM (ONLY_DATABASE_STATS) when --analyze-only is used

Regards,

--
Fujii Masao

Attachments:

v1-0001-vacuumdb-Skip-VACUUM-ONLY_DATABASE_STATS-with-ana.patchapplication/octet-stream; name=v1-0001-vacuumdb-Skip-VACUUM-ONLY_DATABASE_STATS-with-ana.patchDownload
From 7a2dadf3a63367c465fa0eee5d9596db9409555e Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Fri, 8 Aug 2025 15:41:36 +0900
Subject: [PATCH v1] vacuumdb: Skip VACUUM (ONLY_DATABASE_STATS) with
 --analyze-only.

---
 src/bin/scripts/vacuumdb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 79b1096eb08..f755c7b10e7 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -749,7 +749,8 @@ vacuum_one_database(ConnParams *cparams,
 	}
 
 	/* If we used SKIP_DATABASE_STATS, mop up with ONLY_DATABASE_STATS */
-	if (vacopts->skip_database_stats && stage == ANALYZE_NO_STAGE)
+	if (vacopts->skip_database_stats && stage == ANALYZE_NO_STAGE &&
+		!vacopts->analyze_only)
 	{
 		const char *cmd = "VACUUM (ONLY_DATABASE_STATS);";
 		ParallelSlot *free_slot = ParallelSlotsGetIdle(sa, NULL);
-- 
2.50.1

#2Mircea Cadariu
cadariu.mircea@gmail.com
In reply to: Fujii Masao (#1)
Re: vacuumdb --analyze-only does not need to issue VACUUM (ONLY_DATABASE_STATS) ?

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed

Hi,

I tested the patch with log_statement = 'all' and I confirm I do not see the VACUUM log line anymore.

You could consider adding the following representative test in src/bin/scripts/t/100_vacuumdb.pl:
$node->issues_sql_unlike(
[ 'vacuumdb', '--analyze-only', 'postgres' ],
qr/statement: VACUUM.*;/,
'vacuumdb --analyze-only skips vacuum');

Kind regards,
Mircea Cadariu

The new status of this patch is: Waiting on Author

#3Fujii Masao
masao.fujii@gmail.com
In reply to: Mircea Cadariu (#2)
1 attachment(s)
Re: vacuumdb --analyze-only does not need to issue VACUUM (ONLY_DATABASE_STATS) ?

On Tue, Sep 16, 2025 at 10:06 PM Mircea Cadariu
<cadariu.mircea@gmail.com> wrote:

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed

Hi,

I tested the patch with log_statement = 'all' and I confirm I do not see the VACUUM log line anymore.

Thanks for the review and testing!

You could consider adding the following representative test in src/bin/scripts/t/100_vacuumdb.pl:
$node->issues_sql_unlike(
[ 'vacuumdb', '--analyze-only', 'postgres' ],
qr/statement: VACUUM.*;/,
'vacuumdb --analyze-only skips vacuum');

+1. I've added that test to the patch.
Attached is the updated version of the patch.

Regards,

--
Fujii Masao

Attachments:

v2-0001-vacuumdb-Do-not-run-VACUUM-ONLY_DATABASE_STATS-wh.patchapplication/octet-stream; name=v2-0001-vacuumdb-Do-not-run-VACUUM-ONLY_DATABASE_STATS-wh.patchDownload
From b41538e7e35fe0cee2b9233fe9b945739183f1a1 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Tue, 16 Sep 2025 23:21:10 +0900
Subject: [PATCH v2] vacuumdb: Do not run VACUUM (ONLY_DATABASE_STATS) when
 --analyze-only.

Previously, vacuumdb --analyze-only issued VACUUM (ONLY_DATABASE_STATS)
at the end. Since --analyze-only is meant to update optimizer statistics only,
this extra VACUUM command is unnecessary.

This commit prevents vacuumdb --analyze-only from running that redundant
VACUUM command.

Author: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Mircea Cadariu <cadariu.mircea@gmail.com>
Discussion: https://postgr.es/m/CAHGQGwEqHGa-k=wbRMucUVihHVXk4NQkK94GNN=ym9cQ5HBSHg@mail.gmail.com
---
 src/bin/scripts/t/100_vacuumdb.pl | 6 ++++++
 src/bin/scripts/vacuumdb.c        | 3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 945c30df156..a16fad593f7 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -351,5 +351,11 @@ $node->issues_sql_like(
 	],
 	qr/statement: ANALYZE public.parent_table/s,
 	'--analyze-only updates statistics for partitioned tables');
+$node->issues_sql_unlike(
+	[
+		'vacuumdb', '--analyze-only', 'postgres'
+	],
+	qr/statement:\ VACUUM/sx,
+	'--analyze-only does not run vacuum');
 
 done_testing();
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index fd236087e90..6e30f223efe 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -750,7 +750,8 @@ vacuum_one_database(ConnParams *cparams,
 	}
 
 	/* If we used SKIP_DATABASE_STATS, mop up with ONLY_DATABASE_STATS */
-	if (vacopts->skip_database_stats && stage == ANALYZE_NO_STAGE)
+	if (vacopts->skip_database_stats && stage == ANALYZE_NO_STAGE &&
+		!vacopts->analyze_only)
 	{
 		const char *cmd = "VACUUM (ONLY_DATABASE_STATS);";
 		ParallelSlot *free_slot = ParallelSlotsGetIdle(sa, NULL);
-- 
2.50.1

#4Mircea Cadariu
cadariu.mircea@gmail.com
In reply to: Fujii Masao (#3)
Re: vacuumdb --analyze-only does not need to issue VACUUM (ONLY_DATABASE_STATS) ?

Hi,

On Tue, Sep 16, 2025 at 5:36 PM Fujii Masao <masao.fujii@gmail.com> wrote:

Attached is the updated version of the patch.

Thanks for the updated version! LGTM.

Kind regards,
Mircea Cadariu

#5Fujii Masao
masao.fujii@gmail.com
In reply to: Mircea Cadariu (#4)
Re: vacuumdb --analyze-only does not need to issue VACUUM (ONLY_DATABASE_STATS) ?

On Mon, Sep 22, 2025 at 5:56 PM Mircea Cadariu <cadariu.mircea@gmail.com> wrote:

Hi,

On Tue, Sep 16, 2025 at 5:36 PM Fujii Masao <masao.fujii@gmail.com> wrote:

Attached is the updated version of the patch.

Thanks for the updated version! LGTM.

Thanks for the review!
Unless there are any objections, I'll commit the patch.

Regards,

--
Fujii Masao

#6Fujii Masao
masao.fujii@gmail.com
In reply to: Fujii Masao (#5)
Re: vacuumdb --analyze-only does not need to issue VACUUM (ONLY_DATABASE_STATS) ?

On Tue, Sep 23, 2025 at 12:44 AM Fujii Masao <masao.fujii@gmail.com> wrote:

On Mon, Sep 22, 2025 at 5:56 PM Mircea Cadariu <cadariu.mircea@gmail.com> wrote:

Hi,

On Tue, Sep 16, 2025 at 5:36 PM Fujii Masao <masao.fujii@gmail.com> wrote:

Attached is the updated version of the patch.

Thanks for the updated version! LGTM.

Thanks for the review!
Unless there are any objections, I'll commit the patch.

I've pushed the patch. Thanks!

Regards,

--
Fujii Masao