Revive num_dead_tuples column of pg_stat_progress_vacuum

Started by Masahiko Sawadaover 1 year ago15 messages
#1Masahiko Sawada
sawada.mshk@gmail.com
1 attachment(s)

Hi,

Commit 667e65aac3 changed num_dead_tuples and max_dead_tuples columns
to dead_tuple_bytes and max_dead_tuple_bytes columns, respectively.
But at PGConf.dev, I got feedback from multiple people that
num_dead_tuples information still can provide meaning insights for
users to understand the vacuum progress. One use case is to compare
num_dead_tuples to pg_stat_all_tables.n_dead_tup column.

I've attached the patch to revive num_dead_tuples column back to the
pg_stat_progress_vacuum view. This requires to bump catalog version.
We're post-beta1 but it should be okay as it's only for PG17.

Feedback is very welcome.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Attachments:

0001-Revive-num_dead_tuples-column-of-pg_stat_progress_va.patchapplication/octet-stream; name=0001-Revive-num_dead_tuples-column-of-pg_stat_progress_va.patchDownload
From d8a97ccef96b4f4f5ff1398572de255f085612a7 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 4 Jun 2024 06:17:25 +0900
Subject: [PATCH] Revive num_dead_tuples column of pg_stat_progress_vacuum.

Commit 667e65aac3 changed both num_dead_tuples and max_dead_tuples
columns to dead_tuple_bytes and max_dead_tuple_bytes columns,
respectively. But as per discussion, the number of dead tuples
collected still can provide meaningful insights for users.

This change revives num_dead_tuples column of pg_stat_progress_vacuum
view, but not include max_dead_tuples column as it is not predictable.

XXX: bump catalog version.

Reviewed-by:
Discussion: https://postgr.es/m/
---
 doc/src/sgml/monitoring.sgml         | 9 +++++++++
 src/backend/access/heap/vacuumlazy.c | 4 +++-
 src/backend/catalog/system_views.sql | 3 ++-
 src/include/commands/progress.h      | 5 +++--
 src/test/regress/expected/rules.out  | 5 +++--
 5 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 053da8d6e4..062329553e 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6268,6 +6268,15 @@ FROM pg_stat_get_backend_idset() AS backendid;
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+      <structfield>num_dead_tuples</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of dead tuples collected since the last index vacuum cycle.
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>indexes_total</structfield> <type>bigint</type>
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 8145ea8fc3..bed59b72c6 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2887,7 +2887,9 @@ dead_items_add(LVRelState *vacrel, BlockNumber blkno, OffsetNumber *offsets,
 	TidStoreSetBlockOffsets(dead_items, blkno, offsets, num_offsets);
 	vacrel->dead_items_info->num_items += num_offsets;
 
-	/* update the memory usage report */
+	/* update the progress information */
+	pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
+								 vacrel->dead_items_info->num_items);
 	pgstat_progress_update_param(PROGRESS_VACUUM_DEAD_TUPLE_BYTES,
 								 TidStoreMemoryUsage(dead_items));
 }
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 53047cab5f..9880878c7f 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1221,7 +1221,8 @@ CREATE VIEW pg_stat_progress_vacuum AS
         S.param2 AS heap_blks_total, S.param3 AS heap_blks_scanned,
         S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
         S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
-        S.param8 AS indexes_total, S.param9 AS indexes_processed
+        S.param8 AS num_dead_tuples, S.param9 AS indexes_total,
+        S.param10 AS indexes_processed
     FROM pg_stat_get_progress_info('VACUUM') AS S
         LEFT JOIN pg_database D ON S.datid = D.oid;
 
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 82a8fe6bd1..9f10b2c73f 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -25,8 +25,9 @@
 #define PROGRESS_VACUUM_NUM_INDEX_VACUUMS		4
 #define PROGRESS_VACUUM_MAX_DEAD_TUPLE_BYTES	5
 #define PROGRESS_VACUUM_DEAD_TUPLE_BYTES		6
-#define PROGRESS_VACUUM_INDEXES_TOTAL			7
-#define PROGRESS_VACUUM_INDEXES_PROCESSED		8
+#define PROGRESS_VACUUM_NUM_DEAD_TUPLES			7
+#define PROGRESS_VACUUM_INDEXES_TOTAL			8
+#define PROGRESS_VACUUM_INDEXES_PROCESSED		9
 
 /* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
 #define PROGRESS_VACUUM_PHASE_SCAN_HEAP			1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index ef658ad740..820723f6fc 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2052,8 +2052,9 @@ pg_stat_progress_vacuum| SELECT s.pid,
     s.param5 AS index_vacuum_count,
     s.param6 AS max_dead_tuple_bytes,
     s.param7 AS dead_tuple_bytes,
-    s.param8 AS indexes_total,
-    s.param9 AS indexes_processed
+    s.param8 AS num_dead_tuples,
+    s.param9 AS indexes_total,
+    s.param10 AS indexes_processed
    FROM (pg_stat_get_progress_info('VACUUM'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20)
      LEFT JOIN pg_database d ON ((s.datid = d.oid)));
 pg_stat_recovery_prefetch| SELECT stats_reset,
-- 
2.39.3

In reply to: Masahiko Sawada (#1)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Mon, Jun 3, 2024 at 5:27 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I've attached the patch to revive num_dead_tuples column back to the
pg_stat_progress_vacuum view. This requires to bump catalog version.
We're post-beta1 but it should be okay as it's only for PG17.

Feedback is very welcome.

Can we rename this to num_dead_item_ids (or something similar) in
passing? That way we'll avoid confusing the number of dead tuples
removed from the table by VACUUM (which includes dead heap-only
tuples, but excludes any preexisting LP_DEAD items left behind by
opportunistic pruning) with the number of dead item identifiers.

As you know, TIDStore stores TIDs that refer to dead item identifiers
in the heap, which is often very different to the number of dead
tuples removed by VACUUM. The VACUUM log output has reported on dead
item identifiers separately since 14. This seems like a good
opportunity to bring pg_stat_progress_vacuum in line.

--
Peter Geoghegan

#3Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Masahiko Sawada (#1)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On 2024-Jun-03, Masahiko Sawada wrote:

Commit 667e65aac3 changed num_dead_tuples and max_dead_tuples columns
to dead_tuple_bytes and max_dead_tuple_bytes columns, respectively.
But at PGConf.dev, I got feedback from multiple people that
num_dead_tuples information still can provide meaning insights for
users to understand the vacuum progress. One use case is to compare
num_dead_tuples to pg_stat_all_tables.n_dead_tup column.

+1.

@@ -2887,7 +2887,9 @@ dead_items_add(LVRelState *vacrel, BlockNumber blkno, OffsetNumber *offsets,
TidStoreSetBlockOffsets(dead_items, blkno, offsets, num_offsets);
vacrel->dead_items_info->num_items += num_offsets;

-	/* update the memory usage report */
+	/* update the progress information */
+	pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
+								 vacrel->dead_items_info->num_items);
pgstat_progress_update_param(PROGRESS_VACUUM_DEAD_TUPLE_BYTES,
TidStoreMemoryUsage(dead_items));
}

You could use pgstat_progress_update_multi_param here.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"The important things in the world are problems with society that we don't
understand at all. The machines will become more complicated but they won't
be more complicated than the societies that run them." (Freeman Dyson)

#4Andrey M. Borodin
x4mmm@yandex-team.ru
In reply to: Masahiko Sawada (#1)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On 4 Jun 2024, at 00:26, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Thank you! Vacuum enhancement is a really good step forward, and this small change would help a lot of observability tools.

On 4 Jun 2024, at 00:49, Peter Geoghegan <pg@bowt.ie> wrote:

Can we rename this to num_dead_item_ids (or something similar) in
passing?

I do not insist, but many tools will have to adapt to this change [0,1]. However, most of tools will have to deal with removed max_dead_tuples anyway [2]https://github.com/search?q=num_dead_tuples+language%3AGo&amp;type=code, so this is not that big problem.

Best regards, Andrey Borodin.

[0]: https://github.com/jalexandre0/pgmetrics/blob/61cb150f6d1e535513a6a07fc0c6d751fbed517e/collector/collect.go#L1289
[1]: https://github.com/DataDog/integrations-core/blob/250553f3b91d25de28add93afeb929e2abf67e53/postgres/datadog_checks/postgres/util.py#L517
[2]: https://github.com/search?q=num_dead_tuples+language%3AGo&amp;type=code

#5Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Andrey M. Borodin (#4)
1 attachment(s)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Wed, Jun 5, 2024 at 7:19 PM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:

On 4 Jun 2024, at 00:26, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Thank you! Vacuum enhancement is a really good step forward, and this small change would help a lot of observability tools.

On 4 Jun 2024, at 00:49, Peter Geoghegan <pg@bowt.ie> wrote:

Can we rename this to num_dead_item_ids (or something similar) in
passing?

I do not insist, but many tools will have to adapt to this change [0,1]. However, most of tools will have to deal with removed max_dead_tuples anyway [2], so this is not that big problem.

True, this incompatibility would not be a big problem.

num_dead_item_ids seems good to me. I've updated the patch that
incorporated the comment from Álvaro[1]/messages/by-id/202406041535.pmyty3ci4pfd@alvherre.pgsql.

Regards,

[1]: /messages/by-id/202406041535.pmyty3ci4pfd@alvherre.pgsql

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

Attachments:

v2-0001-Reintroduce-dead-tuple-counter-in-pg_stat_progres.patchapplication/x-patch; name=v2-0001-Reintroduce-dead-tuple-counter-in-pg_stat_progres.patchDownload
From 24838307afc52dfc00317579ad88a59ba5c00192 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 4 Jun 2024 06:17:25 +0900
Subject: [PATCH v2] Reintroduce dead tuple counter in pg_stat_progress_vacuum.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Commit 667e65aac3 changed both num_dead_tuples and max_dead_tuples
columns to dead_tuple_bytes and max_dead_tuple_bytes columns,
respectively. But as per discussion, the number of dead tuples
collected still can provide meaningful insights for users.

This change reintroduce the column for the count of dead tuples,
renamed as num_dead_item_ids to avoid confusion with the number of
dead tuples removed by VACUUM, which includes dead heap-only tuples,
but excludes any pre-existing LP_DEAD items left behind by
opportunistic pruning.

XXX: bump catalog version.

Reviewed-by: Peter Geoghegan, Álvaro Herrera, Andrey Borodin
Discussion: https://postgr.es/m/CAD21AoBL5sJE9TRWPyv%2Bw7k5Ee5QAJqDJEDJBUdAaCzGWAdvZw%40mail.gmail.com
---
 doc/src/sgml/monitoring.sgml         |  9 +++++++++
 src/backend/access/heap/vacuumlazy.c | 12 +++++++++---
 src/backend/catalog/system_views.sql |  3 ++-
 src/include/commands/progress.h      |  5 +++--
 src/test/regress/expected/rules.out  |  5 +++--
 5 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 053da8d6e4..b2ad9b446f 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6268,6 +6268,15 @@ FROM pg_stat_get_backend_idset() AS backendid;
       </para></entry>
      </row>
 
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+      <structfield>num_dead_item_ids</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of dead item identifiers collected since the last index vacuum cycle.
+      </para></entry>
+     </row>
+
      <row>
       <entry role="catalog_table_entry"><para role="column_definition">
        <structfield>indexes_total</structfield> <type>bigint</type>
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 8145ea8fc3..ef1df35afa 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -2883,13 +2883,19 @@ dead_items_add(LVRelState *vacrel, BlockNumber blkno, OffsetNumber *offsets,
 			   int num_offsets)
 {
 	TidStore   *dead_items = vacrel->dead_items;
+	const int	prog_index[2] = {
+		PROGRESS_VACUUM_NUM_DEAD_ITEM_IDS,
+		PROGRESS_VACUUM_DEAD_TUPLE_BYTES
+	};
+	int64		prog_val[2];
 
 	TidStoreSetBlockOffsets(dead_items, blkno, offsets, num_offsets);
 	vacrel->dead_items_info->num_items += num_offsets;
 
-	/* update the memory usage report */
-	pgstat_progress_update_param(PROGRESS_VACUUM_DEAD_TUPLE_BYTES,
-								 TidStoreMemoryUsage(dead_items));
+	/* update the progress information */
+	prog_val[0] = vacrel->dead_items_info->num_items;
+	prog_val[1] = TidStoreMemoryUsage(dead_items);
+	pgstat_progress_update_multi_param(2, prog_index, prog_val);
 }
 
 /*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 53047cab5f..efb29adeb3 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1221,7 +1221,8 @@ CREATE VIEW pg_stat_progress_vacuum AS
         S.param2 AS heap_blks_total, S.param3 AS heap_blks_scanned,
         S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
         S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
-        S.param8 AS indexes_total, S.param9 AS indexes_processed
+        S.param8 AS num_dead_item_ids, S.param9 AS indexes_total,
+        S.param10 AS indexes_processed
     FROM pg_stat_get_progress_info('VACUUM') AS S
         LEFT JOIN pg_database D ON S.datid = D.oid;
 
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 82a8fe6bd1..5616d64523 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -25,8 +25,9 @@
 #define PROGRESS_VACUUM_NUM_INDEX_VACUUMS		4
 #define PROGRESS_VACUUM_MAX_DEAD_TUPLE_BYTES	5
 #define PROGRESS_VACUUM_DEAD_TUPLE_BYTES		6
-#define PROGRESS_VACUUM_INDEXES_TOTAL			7
-#define PROGRESS_VACUUM_INDEXES_PROCESSED		8
+#define PROGRESS_VACUUM_NUM_DEAD_ITEM_IDS		7
+#define PROGRESS_VACUUM_INDEXES_TOTAL			8
+#define PROGRESS_VACUUM_INDEXES_PROCESSED		9
 
 /* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
 #define PROGRESS_VACUUM_PHASE_SCAN_HEAP			1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index ef658ad740..13178e2b3d 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2052,8 +2052,9 @@ pg_stat_progress_vacuum| SELECT s.pid,
     s.param5 AS index_vacuum_count,
     s.param6 AS max_dead_tuple_bytes,
     s.param7 AS dead_tuple_bytes,
-    s.param8 AS indexes_total,
-    s.param9 AS indexes_processed
+    s.param8 AS num_dead_item_ids,
+    s.param9 AS indexes_total,
+    s.param10 AS indexes_processed
    FROM (pg_stat_get_progress_info('VACUUM'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20)
      LEFT JOIN pg_database d ON ((s.datid = d.oid)));
 pg_stat_recovery_prefetch| SELECT stats_reset,
-- 
2.39.3

#6Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Masahiko Sawada (#5)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Fri, Jun 7, 2024 at 10:22 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Wed, Jun 5, 2024 at 7:19 PM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:

On 4 Jun 2024, at 00:26, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Thank you! Vacuum enhancement is a really good step forward, and this small change would help a lot of observability tools.

On 4 Jun 2024, at 00:49, Peter Geoghegan <pg@bowt.ie> wrote:

Can we rename this to num_dead_item_ids (or something similar) in
passing?

I do not insist, but many tools will have to adapt to this change [0,1]. However, most of tools will have to deal with removed max_dead_tuples anyway [2], so this is not that big problem.

True, this incompatibility would not be a big problem.

num_dead_item_ids seems good to me. I've updated the patch that
incorporated the comment from Álvaro[1].

I'm going to push the v2 patch in a few days if there is no further comment.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

In reply to: Masahiko Sawada (#5)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Thu, Jun 6, 2024 at 9:23 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

num_dead_item_ids seems good to me. I've updated the patch that
incorporated the comment from Álvaro[1].

Great, thank you.

--
Peter Geoghegan

#8Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Masahiko Sawada (#6)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Tue, Jun 11, 2024 at 10:38 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Jun 7, 2024 at 10:22 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Wed, Jun 5, 2024 at 7:19 PM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:

On 4 Jun 2024, at 00:26, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Thank you! Vacuum enhancement is a really good step forward, and this small change would help a lot of observability tools.

On 4 Jun 2024, at 00:49, Peter Geoghegan <pg@bowt.ie> wrote:

Can we rename this to num_dead_item_ids (or something similar) in
passing?

I do not insist, but many tools will have to adapt to this change [0,1]. However, most of tools will have to deal with removed max_dead_tuples anyway [2], so this is not that big problem.

True, this incompatibility would not be a big problem.

num_dead_item_ids seems good to me. I've updated the patch that
incorporated the comment from Álvaro[1].

I'm going to push the v2 patch in a few days if there is no further comment.

I was about to push the patch but let me confirm just in case: is it
okay to bump the catversion even after post-beta1? This patch
reintroduces a previously-used column to pg_stat_progress_vacuum so it
requires bumping the catversion.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Masahiko Sawada (#8)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

Masahiko Sawada <sawada.mshk@gmail.com> writes:

I was about to push the patch but let me confirm just in case: is it
okay to bump the catversion even after post-beta1?

Yes, that happens somewhat routinely.

regards, tom lane

#10Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#9)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Thu, Jun 13, 2024 at 08:38:05PM -0400, Tom Lane wrote:

Masahiko Sawada <sawada.mshk@gmail.com> writes:

I was about to push the patch but let me confirm just in case: is it
okay to bump the catversion even after post-beta1?

Yes, that happens somewhat routinely.

Up to RC, even after beta2. This happens routinely every year because
tweaks are always required for what got committed. And that's OK to
do so now.
--
Michael

#11Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Michael Paquier (#10)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Fri, Jun 14, 2024 at 9:41 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Jun 13, 2024 at 08:38:05PM -0400, Tom Lane wrote:

Masahiko Sawada <sawada.mshk@gmail.com> writes:

I was about to push the patch but let me confirm just in case: is it
okay to bump the catversion even after post-beta1?

Yes, that happens somewhat routinely.

Up to RC, even after beta2. This happens routinely every year because
tweaks are always required for what got committed. And that's OK to
do so now.

Thank you both for confirmation. I'll push it shortly.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

#12Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Masahiko Sawada (#11)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Fri, Jun 14, 2024 at 9:57 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Jun 14, 2024 at 9:41 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Jun 13, 2024 at 08:38:05PM -0400, Tom Lane wrote:

Masahiko Sawada <sawada.mshk@gmail.com> writes:

I was about to push the patch but let me confirm just in case: is it
okay to bump the catversion even after post-beta1?

Yes, that happens somewhat routinely.

Up to RC, even after beta2. This happens routinely every year because
tweaks are always required for what got committed. And that's OK to
do so now.

Thank you both for confirmation. I'll push it shortly.

Pushed. Thank you for giving feedback and reviewing the patch!

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

#13Robert Treat
rob@xzilla.net
In reply to: Masahiko Sawada (#12)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Thu, Jun 13, 2024 at 9:22 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Jun 14, 2024 at 9:57 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Jun 14, 2024 at 9:41 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Jun 13, 2024 at 08:38:05PM -0400, Tom Lane wrote:

Masahiko Sawada <sawada.mshk@gmail.com> writes:

I was about to push the patch but let me confirm just in case: is it
okay to bump the catversion even after post-beta1?

Yes, that happens somewhat routinely.

Up to RC, even after beta2. This happens routinely every year because
tweaks are always required for what got committed. And that's OK to
do so now.

Thank you both for confirmation. I'll push it shortly.

Pushed. Thank you for giving feedback and reviewing the patch!

One minor side effect of this change is the original idea of comparing
pg_stat_progress.num_dead_tuples to pg_stat_all_tables.n_dead_tup
column becomes less obvious. I presume the release notes for
pg_stat_progress_vacuum will be updated to also include this column
name change as well, so maybe that's enough for folks to figure things
out? At least I couldn't find anywhere in the docs where we have
described the relationship between these columns before. Thoughts?

Robert Treat
https://xzilla.net

#14Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Robert Treat (#13)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Sat, Jun 15, 2024 at 8:47 PM Robert Treat <rob@xzilla.net> wrote:

On Thu, Jun 13, 2024 at 9:22 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Jun 14, 2024 at 9:57 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Jun 14, 2024 at 9:41 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Jun 13, 2024 at 08:38:05PM -0400, Tom Lane wrote:

Masahiko Sawada <sawada.mshk@gmail.com> writes:

I was about to push the patch but let me confirm just in case: is it
okay to bump the catversion even after post-beta1?

Yes, that happens somewhat routinely.

Up to RC, even after beta2. This happens routinely every year because
tweaks are always required for what got committed. And that's OK to
do so now.

Thank you both for confirmation. I'll push it shortly.

Pushed. Thank you for giving feedback and reviewing the patch!

One minor side effect of this change is the original idea of comparing
pg_stat_progress.num_dead_tuples to pg_stat_all_tables.n_dead_tup
column becomes less obvious. I presume the release notes for
pg_stat_progress_vacuum will be updated to also include this column
name change as well, so maybe that's enough for folks to figure things
out?

The release note has been updated, and I think it would help users
understand the change.

At least I couldn't find anywhere in the docs where we have
described the relationship between these columns before. Thoughts?

It would be a good idea to improve the documentation, but I think that
we cannot simply compare these two numbers since the numbers that
these fields count are slightly different. For instance,
pg_stat_all_tables.n_dead_tup includes the number of dead tuples that
are going to be HOT-pruned.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

In reply to: Masahiko Sawada (#14)
Re: Revive num_dead_tuples column of pg_stat_progress_vacuum

On Tue, Jun 18, 2024 at 8:49 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

At least I couldn't find anywhere in the docs where we have
described the relationship between these columns before. Thoughts?

It would be a good idea to improve the documentation, but I think that
we cannot simply compare these two numbers since the numbers that
these fields count are slightly different. For instance,
pg_stat_all_tables.n_dead_tup includes the number of dead tuples that
are going to be HOT-pruned.

This isn't a small difference. It's actually a huge one in many cases:

/messages/by-id/CAH2-WzkkGT2Gt4XauS5eQOQi4mVvL5X49hBTtWccC8DEqeNfKA@mail.gmail.com

Practically speaking they're just two different things, with hardly
any fixed relationship.

--
Peter Geoghegan