Report the number of skipped frozen pages by manual VACUUM

On Fri, Feb 24, 2017 at 1:30 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Hi all,

The autovacuum reports the number of skipped frozen pages to the
VACUUM output. But these information is not appeared by manual VACUUM.
This information is useful for the user to check efficiency of VACUUM.

Attached patch add this information to VACUUM output.

* Example
=# VACUUM VERBOSE test
INFO: vacuuming "public.test"
INFO: "test": found 0 removable, 56 nonremovable row versions in 1
out of 45 pages
DETAIL: 0 dead row versions cannot be removed yet.
There were 0 unused item pointers.
Skipped 0 pages due to buffer pins, 44 frozen pages.
0 pages are entirely empty.
CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s.
VACUUM

I'll register it to next CF.

Added this patch to CF 2017-03.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

I think this is good since the information is useful and it is
a little change.

One thing I'm bothered is that even when the number of frozen page is
one, it will say "1 frozen pages". In other messages, when the
number of page is one, the word "page" rather than "pages" is used
by using ngettext().

In addition, the document (doc/src/sgml/ref/vacuum.sgml) need a modification
to use the new messages in its example.

BTW, this patch can't be applied after the following commit.

commit 9eb344faf54a849898d9be012ddfa8204cfeb57c
Author: Simon Riggs <simon@2ndQuadrant.com>
Date: Fri Mar 3 19:18:25 2017 +0530

Allow vacuums to report oldestxmin

Regards,
Yugo Nagata

On Tue, 28 Feb 2017 16:40:28 +0900
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Feb 24, 2017 at 1:30 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

Hi all,

The autovacuum reports the number of skipped frozen pages to the
VACUUM output. But these information is not appeared by manual VACUUM.
This information is useful for the user to check efficiency of VACUUM.

Attached patch add this information to VACUUM output.

* Example
=# VACUUM VERBOSE test
INFO: vacuuming "public.test"
INFO: "test": found 0 removable, 56 nonremovable row versions in 1
out of 45 pages
DETAIL: 0 dead row versions cannot be removed yet.
There were 0 unused item pointers.
Skipped 0 pages due to buffer pins, 44 frozen pages.
0 pages are entirely empty.
CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s.
VACUUM

I'll register it to next CF.

Added this patch to CF 2017-03.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Yugo Nagata <nagata@sraoss.co.jp>

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Mar 6, 2017 at 12:12 PM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

Hi,

I think this is good since the information is useful and it is
a little change.

Thank you for reviewing this patch!

One thing I'm bothered is that even when the number of frozen page is
one, it will say "1 frozen pages". In other messages, when the
number of page is one, the word "page" rather than "pages" is used
by using ngettext().

I don't think it can say "1 frozen pages" because the number of
skipped pages according to visibility map is always more than 32
(SKIP_PAGES_THRESHOLD).

In addition, the document (doc/src/sgml/ref/vacuum.sgml) need a modification
to use the new messages in its example.

Fixed.

BTW, this patch can't be applied after the following commit.

commit 9eb344faf54a849898d9be012ddfa8204cfeb57c
Author: Simon Riggs <simon@2ndQuadrant.com>
Date: Fri Mar 3 19:18:25 2017 +0530

Allow vacuums to report oldestxmin

Attached updated v2 patch.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On 3/6/17 8:34 PM, Masahiko Sawada wrote:

I don't think it can say "1 frozen pages" because the number of
skipped pages according to visibility map is always more than 32
(SKIP_PAGES_THRESHOLD).

That's just an artifact of how the VM currently works. I'm not a fan of
cross dependencies like that unless there's a pretty good reason.

BTW, I think there's already a function that handles the pluralization
for you. IIRC it's one of the things you can add to an ereport() call.
--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Mar 10, 2017 at 3:58 PM, Jim Nasby <jim.nasby@openscg.com> wrote:

On 3/6/17 8:34 PM, Masahiko Sawada wrote:

I don't think it can say "1 frozen pages" because the number of
skipped pages according to visibility map is always more than 32
(SKIP_PAGES_THRESHOLD).

That's just an artifact of how the VM currently works. I'm not a fan of
cross dependencies like that unless there's a pretty good reason.

Thank you for the comment.
Agreed. Attached fixed version patch.

BTW, I think there's already a function that handles the pluralization for
you. IIRC it's one of the things you can add to an ereport() call.

What is the function name?

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On 3/10/17 5:08 AM, Masahiko Sawada wrote:

BTW, I think there's already a function that handles the pluralization for
you. IIRC it's one of the things you can add to an ereport() call.

What is the function name?

A quick `git grep plural` shows errdetail_plural and errmsg_plural.
--
Jim Nasby, Chief Data Architect, OpenSCG
http://OpenSCG.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, 10 Mar 2017 20:08:42 +0900
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Mar 10, 2017 at 3:58 PM, Jim Nasby <jim.nasby@openscg.com> wrote:

On 3/6/17 8:34 PM, Masahiko Sawada wrote:

I don't think it can say "1 frozen pages" because the number of
skipped pages according to visibility map is always more than 32
(SKIP_PAGES_THRESHOLD).

That's just an artifact of how the VM currently works. I'm not a fan of
cross dependencies like that unless there's a pretty good reason.

Thank you for the comment.
Agreed. Attached fixed version patch.

It seems that it says "frozen pages" if pinskipped_pages is not zero,
rather than about frozenskipped_pages.

How about writing as below?

appendStringInfo(&buf, ngettext("Skipped %u page due to buffer pins, "
"Skipped %u pages due to buffer pins, ",
vacrelstats->pinskipped_pages),
vacrelstats->pinskipped_pages);
appendStringInfo(&buf, ngettext("%u frozen page.\n", "%u frozen pages.\n",
vacrelstats->frozenskipped_pages),
vacrelstats->frozenskipped_pages);

BTW, I think there's already a function that handles the pluralization for
you. IIRC it's one of the things you can add to an ereport() call.

What is the function name?

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

--
Yugo Nagata <nagata@sraoss.co.jp>

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Mar 15, 2017 at 1:09 PM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 10 Mar 2017 20:08:42 +0900
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Mar 10, 2017 at 3:58 PM, Jim Nasby <jim.nasby@openscg.com> wrote:

On 3/6/17 8:34 PM, Masahiko Sawada wrote:

I don't think it can say "1 frozen pages" because the number of
skipped pages according to visibility map is always more than 32
(SKIP_PAGES_THRESHOLD).

That's just an artifact of how the VM currently works. I'm not a fan of
cross dependencies like that unless there's a pretty good reason.

Thank you for the comment.
Agreed. Attached fixed version patch.

It seems that it says "frozen pages" if pinskipped_pages is not zero,
rather than about frozenskipped_pages.

How about writing as below?

appendStringInfo(&buf, ngettext("Skipped %u page due to buffer pins, "
"Skipped %u pages due to buffer pins, ",
vacrelstats->pinskipped_pages),
vacrelstats->pinskipped_pages);
appendStringInfo(&buf, ngettext("%u frozen page.\n", "%u frozen pages.\n",
vacrelstats->frozenskipped_pages),
vacrelstats->frozenskipped_pages);

Yeah, you're right. I've attached the updated version patch
incorporated your comment and fixing documentation.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Wed, Mar 15, 2017 at 7:51 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Wed, Mar 15, 2017 at 1:09 PM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 10 Mar 2017 20:08:42 +0900
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Mar 10, 2017 at 3:58 PM, Jim Nasby <jim.nasby@openscg.com> wrote:

On 3/6/17 8:34 PM, Masahiko Sawada wrote:

I don't think it can say "1 frozen pages" because the number of
skipped pages according to visibility map is always more than 32
(SKIP_PAGES_THRESHOLD).

That's just an artifact of how the VM currently works. I'm not a fan of
cross dependencies like that unless there's a pretty good reason.

Thank you for the comment.
Agreed. Attached fixed version patch.

It seems that it says "frozen pages" if pinskipped_pages is not zero,
rather than about frozenskipped_pages.

How about writing as below?

appendStringInfo(&buf, ngettext("Skipped %u page due to buffer pins, "
"Skipped %u pages due to buffer pins, ",
vacrelstats->pinskipped_pages),
vacrelstats->pinskipped_pages);
appendStringInfo(&buf, ngettext("%u frozen page.\n", "%u frozen pages.\n",
vacrelstats->frozenskipped_pages),
vacrelstats->frozenskipped_pages);

Yeah, you're right. I've attached the updated version patch
incorporated your comment and fixing documentation.

The patch looks very simple and good to me.
Barring objection, I will commit this.

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Mar 23, 2017 at 4:28 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Wed, Mar 15, 2017 at 7:51 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Wed, Mar 15, 2017 at 1:09 PM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 10 Mar 2017 20:08:42 +0900
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Mar 10, 2017 at 3:58 PM, Jim Nasby <jim.nasby@openscg.com> wrote:

On 3/6/17 8:34 PM, Masahiko Sawada wrote:

I don't think it can say "1 frozen pages" because the number of
skipped pages according to visibility map is always more than 32
(SKIP_PAGES_THRESHOLD).

That's just an artifact of how the VM currently works. I'm not a fan of
cross dependencies like that unless there's a pretty good reason.

Thank you for the comment.
Agreed. Attached fixed version patch.

It seems that it says "frozen pages" if pinskipped_pages is not zero,
rather than about frozenskipped_pages.

How about writing as below?

appendStringInfo(&buf, ngettext("Skipped %u page due to buffer pins, "
"Skipped %u pages due to buffer pins, ",
vacrelstats->pinskipped_pages),
vacrelstats->pinskipped_pages);
appendStringInfo(&buf, ngettext("%u frozen page.\n", "%u frozen pages.\n",
vacrelstats->frozenskipped_pages),
vacrelstats->frozenskipped_pages);

Yeah, you're right. I've attached the updated version patch
incorporated your comment and fixing documentation.

The patch looks very simple and good to me.
Barring objection, I will commit this.

Pushed.

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sat, Mar 25, 2017 at 2:42 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Mar 23, 2017 at 4:28 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Wed, Mar 15, 2017 at 7:51 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Wed, Mar 15, 2017 at 1:09 PM, Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 10 Mar 2017 20:08:42 +0900
Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Fri, Mar 10, 2017 at 3:58 PM, Jim Nasby <jim.nasby@openscg.com> wrote:

On 3/6/17 8:34 PM, Masahiko Sawada wrote:

I don't think it can say "1 frozen pages" because the number of
skipped pages according to visibility map is always more than 32
(SKIP_PAGES_THRESHOLD).

That's just an artifact of how the VM currently works. I'm not a fan of
cross dependencies like that unless there's a pretty good reason.

Thank you for the comment.
Agreed. Attached fixed version patch.

It seems that it says "frozen pages" if pinskipped_pages is not zero,
rather than about frozenskipped_pages.

How about writing as below?

appendStringInfo(&buf, ngettext("Skipped %u page due to buffer pins, "
"Skipped %u pages due to buffer pins, ",
vacrelstats->pinskipped_pages),
vacrelstats->pinskipped_pages);
appendStringInfo(&buf, ngettext("%u frozen page.\n", "%u frozen pages.\n",
vacrelstats->frozenskipped_pages),
vacrelstats->frozenskipped_pages);

Yeah, you're right. I've attached the updated version patch
incorporated your comment and fixing documentation.

The patch looks very simple and good to me.
Barring objection, I will commit this.

Pushed.

Thank you!
Actually, vacuum execution example in doc is still inconsistent with
actual output because commit 9eb344faf54a849898d9be012ddfa8204cfeb57c
added the oldest xmin to the vacuum verbose log. I mentioned another
thread[1]/messages/by-id/CAD21AoAGA2pB3p-CWmTkxBsbkZS1bcDGBLcYVcvcDxspG_XAfA@mail.gmail.com but it's still inconsistent.

[1]: /messages/by-id/CAD21AoAGA2pB3p-CWmTkxBsbkZS1bcDGBLcYVcvcDxspG_XAfA@mail.gmail.com

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers