Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Wed, May 8, 2019 at 02:14:04AM +0900, Fujii Masao wrote: > On Tue, May 7, 2019 at 5:33 PM Masahiko Sawada wrote: > > Sorry for the late. I've reviewed the patch and it looks good to me. > > Thanks for the review! I committed the patch. Great. I noticed from the release notes that it was odd we could control this via a CREATE TABLE option but not VACUUM. I have updated the release notes. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 8:15 PM Julien Rouhaud wrote: > > On Mon, Apr 8, 2019 at 12:22 PM Fujii Masao wrote: > > > > On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada > > wrote: > > > > > > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao wrote: > > > > > > > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud > > > > wrote: > > > > > > > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao > > > > > wrote: > > > > > > > > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki > > > > > > : > > > > > >> > > > > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > > > > > >> > "vacuum_truncate" gets my vote too. > > > > > >> > > > > > >> +1 > > > > > > > > > > > > > > > > > > +1 > > > > > > ISTM that we have small consensus to > > > > > > use "vacuum_truncate". > > > > > > > > > > I'm also fine with this name, and I also saw reports that this option > > > > > is already needed in some production workload, as Andres explained. > > > > > > > > OK, so I pushed the "vacuum_truncate" version of the patch. > > > > > > Thank you! > > > > > > "TRUNCATE" option for vacuum command should be added to the open items? > > > > Yes, I think. > > Attached is the patch which adds TRUNCATE option into VACUUM. > > Thanks. > > I just reviewed the patch, it works as expected, no special comment on the > code. > > Minor nitpicking: > > - lock on the table. > + lock on the table. The TRUNCATE parameter > + to , if specified, overrides the value > + of this option. > > maybe "parameter of" instead of "parameter to"? Thanks for the review! I changed the doc that way. Regards, -- Fujii Masao
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Tue, May 7, 2019 at 5:33 PM Masahiko Sawada wrote: > > On Mon, Apr 8, 2019 at 7:29 PM Masahiko Sawada wrote: > > > > On Mon, Apr 8, 2019 at 7:22 PM Fujii Masao wrote: > > > > > > On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada > > > wrote: > > > > > > > > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao > > > > wrote: > > > > > > > > > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud > > > > > wrote: > > > > > > > > > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao > > > > > > wrote: > > > > > > > > > > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki > > > > > > > : > > > > > > >> > > > > > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > > > > > > >> > "vacuum_truncate" gets my vote too. > > > > > > >> > > > > > > >> +1 > > > > > > > > > > > > > > > > > > > > > +1 > > > > > > > ISTM that we have small consensus to > > > > > > > use "vacuum_truncate". > > > > > > > > > > > > I'm also fine with this name, and I also saw reports that this > > > > > > option > > > > > > is already needed in some production workload, as Andres explained. > > > > > > > > > > OK, so I pushed the "vacuum_truncate" version of the patch. > > > > > > > > Thank you! > > > > > > > > "TRUNCATE" option for vacuum command should be added to the open items? > > > > > > Yes, I think. > > > > Added. > > > > > Attached is the patch which adds TRUNCATE option into VACUUM. > > > > Thank you for the patch! I will review it. > > > > Sorry for the late. I've reviewed the patch and it looks good to me. Thanks for the review! I committed the patch. Regards, -- Fujii Masao
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 7:29 PM Masahiko Sawada wrote: > > On Mon, Apr 8, 2019 at 7:22 PM Fujii Masao wrote: > > > > On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada > > wrote: > > > > > > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao wrote: > > > > > > > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud > > > > wrote: > > > > > > > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao > > > > > wrote: > > > > > > > > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki > > > > > > : > > > > > >> > > > > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > > > > > >> > "vacuum_truncate" gets my vote too. > > > > > >> > > > > > >> +1 > > > > > > > > > > > > > > > > > > +1 > > > > > > ISTM that we have small consensus to > > > > > > use "vacuum_truncate". > > > > > > > > > > I'm also fine with this name, and I also saw reports that this option > > > > > is already needed in some production workload, as Andres explained. > > > > > > > > OK, so I pushed the "vacuum_truncate" version of the patch. > > > > > > Thank you! > > > > > > "TRUNCATE" option for vacuum command should be added to the open items? > > > > Yes, I think. > > Added. > > > Attached is the patch which adds TRUNCATE option into VACUUM. > > Thank you for the patch! I will review it. > Sorry for the late. I've reviewed the patch and it looks good to me. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi, On 2019-04-08 19:22:04 +0900, Fujii Masao wrote: > On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada wrote: > > "TRUNCATE" option for vacuum command should be added to the open items? > > Yes, I think. > Attached is the patch which adds TRUNCATE option into VACUUM. This has been an open item for about three weeks. Please work on resolving this soon. - Andres
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
At Wed, 10 Apr 2019 02:00:03 +0900, Fujii Masao wrote in > On Tue, Apr 9, 2019 at 1:07 PM Kyotaro HORIGUCHI > wrote: > > > > Hello. > > > > At Mon, 8 Apr 2019 19:22:04 +0900, Fujii Masao > > wrote in > > > > > > "TRUNCATE" option for vacuum command should be added to the open items? > > > > > > Yes, I think. > > > Attached is the patch which adds TRUNCATE option into VACUUM. > > > > By the way, this might have been discussed upthread, but boolean > > options of VACUUM command is defaulted to true. So, it seems to > > me that the name is better (or convenient to me) to be > > SKIP_TRUNCATE. The name of the reloption seems good to me. > > If we really use SKIP_TRUNCATE, we need to use SKIP_INDEX_CLEANUP > instead of INDEX_CLEANUP for the consistency. But TRUNCATE and > INDEX_CLEANUP look more intuitive to me than SKIP_TRUNCATE and > SKIP_INDEX_CLEANUP. Ah, yes, we already have INDEX_CLEANUP. I'm fine with TRUNCATE. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Tue, Apr 9, 2019 at 1:07 PM Kyotaro HORIGUCHI wrote: > > Hello. > > At Mon, 8 Apr 2019 19:22:04 +0900, Fujii Masao wrote > in > > > "TRUNCATE" option for vacuum command should be added to the open items? > > > > Yes, I think. > > Attached is the patch which adds TRUNCATE option into VACUUM. > > By the way, this might have been discussed upthread, but boolean > options of VACUUM command is defaulted to true. So, it seems to > me that the name is better (or convenient to me) to be > SKIP_TRUNCATE. The name of the reloption seems good to me. If we really use SKIP_TRUNCATE, we need to use SKIP_INDEX_CLEANUP instead of INDEX_CLEANUP for the consistency. But TRUNCATE and INDEX_CLEANUP look more intuitive to me than SKIP_TRUNCATE and SKIP_INDEX_CLEANUP. Regards, -- Fujii Masao
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hello. At Mon, 8 Apr 2019 19:22:04 +0900, Fujii Masao wrote in > > "TRUNCATE" option for vacuum command should be added to the open items? > > Yes, I think. > Attached is the patch which adds TRUNCATE option into VACUUM. By the way, this might have been discussed upthread, but boolean options of VACUUM command is defaulted to true. So, it seems to me that the name is better (or convenient to me) to be SKIP_TRUNCATE. The name of the reloption seems good to me. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Fujii Masao [mailto:masao.fu...@gmail.com] > Thanks for the info, so I marked the patch as committed. Thanks a lot for your hard work! This felt relatively tough despite the simplicity of the patch. I'm starting to feel the difficulty and fatigue in developing in the community... Regards Takayuki Tsunakawa
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 10:24 AM Fujii Masao wrote: > > On Mon, Apr 8, 2019 at 5:20 PM Julien Rouhaud wrote: > > > > On Mon, Apr 8, 2019 at 10:15 AM Fujii Masao wrote: > > > > > > But it has not been actually pushed into the community's git > > > repository yet.That's maybe because it's been a while since > > > my last commit and my commit bit is temporarily limited? > > > Anyway the patch has been pushed before April 8th 0:00 in AoE. > > > > Indeed, there's no mail yet, but I can see the commit at > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=119dcfad988d5b5d9f52b256087869997670aa36 > > Thanks for the info, so I marked the patch as committed. FTR I just received the notification email, and it's also up at https://www.postgresql.org/message-id/E1hDOzB-pO-ED%40gemulon.postgresql.org!
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 12:22 PM Fujii Masao wrote: > > On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada wrote: > > > > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao wrote: > > > > > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud wrote: > > > > > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao > > > > wrote: > > > > > > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki > > > > > : > > > > >> > > > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > > > > >> > "vacuum_truncate" gets my vote too. > > > > >> > > > > >> +1 > > > > > > > > > > > > > > > +1 > > > > > ISTM that we have small consensus to > > > > > use "vacuum_truncate". > > > > > > > > I'm also fine with this name, and I also saw reports that this option > > > > is already needed in some production workload, as Andres explained. > > > > > > OK, so I pushed the "vacuum_truncate" version of the patch. > > > > Thank you! > > > > "TRUNCATE" option for vacuum command should be added to the open items? > > Yes, I think. > Attached is the patch which adds TRUNCATE option into VACUUM. Thanks. I just reviewed the patch, it works as expected, no special comment on the code. Minor nitpicking: - lock on the table. + lock on the table. The TRUNCATE parameter + to , if specified, overrides the value + of this option. maybe "parameter of" instead of "parameter to"?
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 7:22 PM Fujii Masao wrote: > > On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada wrote: > > > > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao wrote: > > > > > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud wrote: > > > > > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao > > > > wrote: > > > > > > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki > > > > > : > > > > >> > > > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > > > > >> > "vacuum_truncate" gets my vote too. > > > > >> > > > > >> +1 > > > > > > > > > > > > > > > +1 > > > > > ISTM that we have small consensus to > > > > > use "vacuum_truncate". > > > > > > > > I'm also fine with this name, and I also saw reports that this option > > > > is already needed in some production workload, as Andres explained. > > > > > > OK, so I pushed the "vacuum_truncate" version of the patch. > > > > Thank you! > > > > "TRUNCATE" option for vacuum command should be added to the open items? > > Yes, I think. Added. > Attached is the patch which adds TRUNCATE option into VACUUM. Thank you for the patch! I will review it. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada wrote: > > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao wrote: > > > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud wrote: > > > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao wrote: > > > > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki : > > > >> > > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > > > >> > "vacuum_truncate" gets my vote too. > > > >> > > > >> +1 > > > > > > > > > > > > +1 > > > > ISTM that we have small consensus to > > > > use "vacuum_truncate". > > > > > > I'm also fine with this name, and I also saw reports that this option > > > is already needed in some production workload, as Andres explained. > > > > OK, so I pushed the "vacuum_truncate" version of the patch. > > Thank you! > > "TRUNCATE" option for vacuum command should be added to the open items? Yes, I think. Attached is the patch which adds TRUNCATE option into VACUUM. Regards, -- Fujii Masao vacuum_truncate_v1.patch Description: Binary data
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao wrote: > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud wrote: > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao wrote: > > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki : > > >> > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > > >> > "vacuum_truncate" gets my vote too. > > >> > > >> +1 > > > > > > > > > +1 > > > ISTM that we have small consensus to > > > use "vacuum_truncate". > > > > I'm also fine with this name, and I also saw reports that this option > > is already needed in some production workload, as Andres explained. > > OK, so I pushed the "vacuum_truncate" version of the patch. Thank you! "TRUNCATE" option for vacuum command should be added to the open items? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 5:20 PM Julien Rouhaud wrote: > > On Mon, Apr 8, 2019 at 10:15 AM Fujii Masao wrote: > > > > OK, so I pushed the "vacuum_truncate" version of the patch. > > Thanks! > > > But it has not been actually pushed into the community's git > > repository yet.That's maybe because it's been a while since > > my last commit and my commit bit is temporarily limited? > > Anyway the patch has been pushed before April 8th 0:00 in AoE. > > Indeed, there's no mail yet, but I can see the commit at > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=119dcfad988d5b5d9f52b256087869997670aa36 Thanks for the info, so I marked the patch as committed. Regards, -- Fujii Masao
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 10:15 AM Fujii Masao wrote: > > OK, so I pushed the "vacuum_truncate" version of the patch. Thanks! > But it has not been actually pushed into the community's git > repository yet.That's maybe because it's been a while since > my last commit and my commit bit is temporarily limited? > Anyway the patch has been pushed before April 8th 0:00 in AoE. Indeed, there's no mail yet, but I can see the commit at https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=119dcfad988d5b5d9f52b256087869997670aa36
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud wrote: > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao wrote: > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki : > >> > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > >> > "vacuum_truncate" gets my vote too. > >> > >> +1 > > > > > > +1 > > ISTM that we have small consensus to > > use "vacuum_truncate". > > I'm also fine with this name, and I also saw reports that this option > is already needed in some production workload, as Andres explained. OK, so I pushed the "vacuum_truncate" version of the patch. But it has not been actually pushed into the community's git repository yet.That's maybe because it's been a while since my last commit and my commit bit is temporarily limited? Anyway the patch has been pushed before April 8th 0:00 in AoE. Regards, -- Fujii Masao
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao wrote: > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki : >> >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] >> > "vacuum_truncate" gets my vote too. >> >> +1 > > > +1 > ISTM that we have small consensus to > use "vacuum_truncate". I'm also fine with this name, and I also saw reports that this option is already needed in some production workload, as Andres explained.
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
2019年4月8日(月) 14:22 Tsunakawa, Takayuki : > From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > > "vacuum_truncate" gets my vote too. > > +1 > +1 ISTM that we have small consensus to use "vacuum_truncate". regards,
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > "vacuum_truncate" gets my vote too. +1 From: 'Andres Freund' [mailto:and...@anarazel.de] > Personally I think the name just needs some committer to make a > call. This largely is going to be used after encountering too many > cancellations in production, and researching the cause. Minor spelling > differences don't seem to particularly matter here. Absolutely. Thank you. From: 'Andres Freund' [mailto:and...@anarazel.de] > I think it needs to be an autovac option. The production issue is that > autovacuums constantly cancel queries on the standbys despite > hot_standby_feedback if you have a workload that does frequent > truncations. If there's no way to configure it in a way that autovacuum > takes it into account, people will just disable autovacuum and rely > entirely on manual scripts. That's what already happens - leading to a > much bigger foot-gun than disabling truncation. FWIW, people already in > production use the workaround to configuring snapshot_too_old as that, > for undocumented reasons, disables trunctations. That's not better > either. Right. We don't want autovacuum to be considered as a criminal. Regards Takayuki Tsunakawa
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 2019-Apr-08, Tom Lane wrote: > The closest match to that name, probably, is just "vacuum_truncate" --- > which was proposed at the beginning of March, but apparently also > rejected, because there's no subsequent reference. "vacuum_truncate" gets my vote too. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 08, 2019 at 03:56:52AM +, Tsunakawa, Takayuki wrote: > Consensus on the name seems to use truncate rather than shrink (a > few poople kindly said they like shrink, and I'm OK with either > name.) And there's no dispute on the behavior. Do you see any > other point? I personally much prefer "truncate" than "shrink", which was my initial opinion in upthread as well. Please note that the feature freeze will be effective in just a bit more than 7 hours, as of the 8th of April 0AM on the AoE timezone (Anywhere on Earth). -- Michael signature.asc Description: PGP signature
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi, On 2019-04-08 00:38:44 -0400, Tom Lane wrote: > "Tsunakawa, Takayuki" writes: > > From: Tom Lane [mailto:t...@sss.pgh.pa.us] > >> And, as far as I can see from a quick review of the thread, > >> we don't really have consensus on the names and behaviors. Personally I think the name just needs some committer to make a call. This largely is going to be used after encountering too many cancellations in production, and researching the cause. Minor spelling differences don't seem to particularly matter here. > My own dog in this fight is that we shouldn't have the option at all, > never mind what the name is. A persistent reloption to disable truncation > seems like a real foot-gun. I'd be okay with a VACUUM command option, > but for some reason that isn't there at all. I think it needs to be an autovac option. The production issue is that autovacuums constantly cancel queries on the standbys despite hot_standby_feedback if you have a workload that does frequent truncations. If there's no way to configure it in a way that autovacuum takes it into account, people will just disable autovacuum and rely entirely on manual scripts. That's what already happens - leading to a much bigger foot-gun than disabling truncation. FWIW, people already in production use the workaround to configuring snapshot_too_old as that, for undocumented reasons, disables trunctations. That's not better either. Greetings, Andres Freund
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
"Tsunakawa, Takayuki" writes: > From: Tom Lane [mailto:t...@sss.pgh.pa.us] >> And, as far as I can see from a quick review of the thread, >> we don't really have consensus on the names and behaviors. > Consensus on the name seems to use truncate rather than shrink (a few poople > kindly said they like shrink, and I'm OK with either name.) And there's no > dispute on the behavior. Do you see any other point? The last patch uses the name vacuum_truncate_enabled, which so far as I can see never appeared in the thread before today. How can you claim there's consensus for that? I see references back in February to truncate_enabled and vacuum_enabled, but there was certainly no consensus for either, seeing how long the thread has dragged on since then (those references are barely halfway down the thread). Pasting them together to make a carpal-tunnel-inducing name isn't automatically going to satisfy people. Also, it looks like one of the main bones of contention is whether the option is named consistently with the index-scan-disable option, which seems to have ended up named "vacuum_index_cleanup". I submit that "vacuum_truncate_enabled" is not consistent with that; it's not even the same part of speech. The closest match to that name, probably, is just "vacuum_truncate" --- which was proposed at the beginning of March, but apparently also rejected, because there's no subsequent reference. My own dog in this fight is that we shouldn't have the option at all, never mind what the name is. A persistent reloption to disable truncation seems like a real foot-gun. I'd be okay with a VACUUM command option, but for some reason that isn't there at all. regards, tom lane
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Tom Lane [mailto:t...@sss.pgh.pa.us] > And, as far as I can see from a quick review of the thread, > we don't really have consensus on the names and behaviors. Consensus on the name seems to use truncate rather than shrink (a few poople kindly said they like shrink, and I'm OK with either name.) And there's no dispute on the behavior. Do you see any other point? Regards Takayuki Tsunakawa
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
"Tsunakawa, Takayuki" writes: > From: Andres Freund [mailto:and...@anarazel.de] >> I hope you realize feature freeze is in a few hours... > Ouch! Could you take care of committing the patch, please? I wouldn't be > able to express the sadness and tiredness just in case this is pushed to 13 > because of the parameter name... As far as I can see, the entire intellectual content of this patch is in the names and behaviors of the user-visible options; there's certainly no significant amount of new logic outside of that. And, as far as I can see from a quick review of the thread, we don't really have consensus on the names and behaviors. So I think forcing this in a few hours before feature freeze is a bad idea. That isn't going to create consensus where there was none before; it will just annoy people. regards, tom lane
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi Andres, Fujii-san, any committer, From: Andres Freund [mailto:and...@anarazel.de] > On 2019-04-08 09:52:27 +0900, Fujii Masao wrote: > > I'm thinking to commit this patch at first. We can change the term > > and add the support of "TRUNCATE" option for VACUUM command later. > > I hope you realize feature freeze is in a few hours... Ouch! Could you take care of committing the patch, please? I wouldn't be able to express the sadness and tiredness just in case this is pushed to 13 because of the parameter name... Regards Takayuki Tsunakawa
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi, On 2019-04-08 09:52:27 +0900, Fujii Masao wrote: > I'm thinking to commit this patch at first. We can change the term > and add the support of "TRUNCATE" option for VACUUM command later. I hope you realize feature freeze is in a few hours... Greetings, Andres Freund
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Apr 8, 2019 at 9:52 AM Fujii Masao wrote: > > On Sat, Apr 6, 2019 at 2:04 AM Robert Haas wrote: > > > > On Thu, Apr 4, 2019 at 9:19 PM Masahiko Sawada > > wrote: > > > As INDEX_CLEANUP option has been added by commit a96c41f, the new > > > option for this feature could also accept zero or one boolean > > > argument, that is SHRINK_TABLE [true|false] and true by default. > > > Explicit options on VACUUM command overwrite options set by > > > reloptions. And if the boolean argument is omitted the option depends > > > on the reloptions. > > > > Yes, I think that's how it should work, because that's how the other > > option works, and there's no compelling reason to be consistent. > > > > My preference is for "truncate" over "shrink". > > +1 > > Attached is the updated version of the patch. > I just replaced "shrink" with "truncate" and rebased the patch > on the master. Thank you for updating the patch! "vacuum_truncate" option would be more consistent with vacuum_index_cleanup option rather than "vacuum_truncate_enabled' option? > I'm thinking to commit this patch at first. > We can change the term and add the support of "TRUNCATE" option > for VACUUM command later. +1 Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Sat, Apr 6, 2019 at 2:04 AM Robert Haas wrote: > > On Thu, Apr 4, 2019 at 9:19 PM Masahiko Sawada wrote: > > As INDEX_CLEANUP option has been added by commit a96c41f, the new > > option for this feature could also accept zero or one boolean > > argument, that is SHRINK_TABLE [true|false] and true by default. > > Explicit options on VACUUM command overwrite options set by > > reloptions. And if the boolean argument is omitted the option depends > > on the reloptions. > > Yes, I think that's how it should work, because that's how the other > option works, and there's no compelling reason to be consistent. > > My preference is for "truncate" over "shrink". +1 Attached is the updated version of the patch. I just replaced "shrink" with "truncate" and rebased the patch on the master. I'm thinking to commit this patch at first. We can change the term and add the support of "TRUNCATE" option for VACUUM command later. Regards, -- Fujii Masao disable-vacuum-truncation_v7.patch Description: Binary data
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Fri, Apr 5, 2019 at 3:28 PM Robert Haas wrote: > > On Fri, Apr 5, 2019 at 2:11 PM Julien Rouhaud wrote: > > > My preference is for "truncate" over "shrink". > > > > I don't really like "shrink" either, but users already have problems > > to get the difference between VACUUM and VACUUM FULL, I'm afraid that > > "VACUUM TRUNCATE_TABLE" will just make things worse. > > That's not the proposed syntax, though. What you'd end up writing is > something like VACUUM (TRUNCATE OFF) myrel. > +1 for this syntax. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
The following review has been posted through the commitfest application: make installcheck-world: not tested Implements feature: not tested Spec compliant: not tested Documentation:not tested I have read disable-vacuum-truncation_v6.patch. I like it the way it is. Word "truncation" for me means "remove everything from the table" (as in TRUNCATE) and I don't want VACUUM to do that :), shrink feels appropriate. TRIM has a connotation from SSD drives, where you can discard blocks in the middle of your data and let the underlying infrastructure handle space allocation on it. Please keep it for the good times Postgres can punch a hole in the middle of relation and let filesystem handle that space. The new status of this patch is: Ready for Committer
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Fri, Apr 5, 2019 at 3:28 PM Adrien Mobile wrote: > How about TRIM? The problem, in my view, is not that there is anything ipso facto wrong with SHRINK. The problem is that it's a turn term that has only de minimis use up until now. Replacing it with some other term that has never before been used to refer to the behavior under discussion does not, in my view, fix anything. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Le 5 avril 2019 20:11:38 GMT+02:00, Julien Rouhaud a écrit : >On Fri, Apr 5, 2019 at 7:04 PM Robert Haas >wrote: >> > >> My preference is for "truncate" over "shrink". > >I don't really like "shrink" either, but users already have problems >to get the difference between VACUUM and VACUUM FULL, I'm afraid that >"VACUUM TRUNCATE_TABLE" will just make things worse. How about TRIM? -- Sent from my Android phone with K-9 Mail. Please excuse my brevity.
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Fri, Apr 5, 2019 at 2:11 PM Julien Rouhaud wrote: > > My preference is for "truncate" over "shrink". > > I don't really like "shrink" either, but users already have problems > to get the difference between VACUUM and VACUUM FULL, I'm afraid that > "VACUUM TRUNCATE_TABLE" will just make things worse. That's not the proposed syntax, though. What you'd end up writing is something like VACUUM (TRUNCATE OFF) myrel. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Fri, Apr 5, 2019 at 7:04 PM Robert Haas wrote: > > On Thu, Apr 4, 2019 at 9:19 PM Masahiko Sawada wrote: > > As INDEX_CLEANUP option has been added by commit a96c41f, the new > > option for this feature could also accept zero or one boolean > > argument, that is SHRINK_TABLE [true|false] and true by default. > > Explicit options on VACUUM command overwrite options set by > > reloptions. And if the boolean argument is omitted the option depends > > on the reloptions. > > Yes, I think that's how it should work, because that's how the other > option works, and there's no compelling reason to be consistent. Indeed, I totally agree. > My preference is for "truncate" over "shrink". I don't really like "shrink" either, but users already have problems to get the difference between VACUUM and VACUUM FULL, I'm afraid that "VACUUM TRUNCATE_TABLE" will just make things worse.
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Thu, Apr 4, 2019 at 9:19 PM Masahiko Sawada wrote: > As INDEX_CLEANUP option has been added by commit a96c41f, the new > option for this feature could also accept zero or one boolean > argument, that is SHRINK_TABLE [true|false] and true by default. > Explicit options on VACUUM command overwrite options set by > reloptions. And if the boolean argument is omitted the option depends > on the reloptions. Yes, I think that's how it should work, because that's how the other option works, and there's no compelling reason to be consistent. My preference is for "truncate" over "shrink". -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Thu, Apr 4, 2019 at 10:07 PM Julien Rouhaud wrote: > > On Thu, Apr 4, 2019 at 1:23 PM Masahiko Sawada wrote: > > > > On Thu, Apr 4, 2019 at 1:26 PM Tsunakawa, Takayuki > > wrote: > > > > > > From: Fujii Masao [mailto:masao.fu...@gmail.com] > > > > reloption for TOAST is also required? > > > > > > # I've come back to the office earlier than planned... > > > > > > Hm, there's no reason to not provide toast.vacuum_shrink_enabled. Done > > > with the attached patch. > > > > > > > Thank you for updating the patch! > > +1! > > > +vacuum_shrink_enabled, > > toast.vacuum_shrink_enabled > > (boolean) > > + > > + > > + Enables or disables shrinking the table when it's vacuumed. > > + This also applies to autovacuum. > > + The default is true. If true, VACUUM frees empty pages at the > > end of the table. > > > > "VACUUM" needs or "vacuum" is more appropriate here? > > also, the documentation should point out that freeing is not > guaranteed. Something like > > + The default is true. If true, VACUUM will try to free empty > pages at the end of the table. +1 > > > I'm not sure the consensus we got here but we don't make the vacuum > > command option for this? > > I don't think here's a clear consensus, but my personal vote is to add > it, with SHRINK_TABLE = [ force_on | force_off | default ] (unless a > better proposal has been made already) As INDEX_CLEANUP option has been added by commit a96c41f, the new option for this feature could also accept zero or one boolean argument, that is SHRINK_TABLE [true|false] and true by default. Explicit options on VACUUM command overwrite options set by reloptions. And if the boolean argument is omitted the option depends on the reloptions. FWIW, I also would like to defer to committer on the naming new option but an another possible comment on that could be that the term 'truncate' might be more suitable rather than 'shrink' in the context of lazy vacuum. As Tsunakawa-san mentioned the term 'shrink' is used in PostgreSQL documentation but we use it mostly in the context of VACUUM FULL. I found two paragraphs that use the term 'shrink'. vacuum.sgml: The FULL option is not recommended for routine use, but might be useful in special cases. An example is when you have deleted or updated most of the rows in a table and would like the table to physically shrink to occupy less disk space and allow faster table scans. VACUUM FULL will usually shrink the table more than a plain VACUUM would. maintenance.sgml Although VACUUM FULL can be used to shrink a table back to its minimum size and return the disk space to the operating system, there is not much point in this if the table will just grow again in the future. Thus, moderately-frequent standard VACUUM runs are a better approach than infrequent VACUUM FULL runs for maintaining heavily-updated tables. On the other hand, we use the term 'truncate' in the progress reporting of lazy vacuum (see documentation of pg_stat_progress_vacuum). So I'm concerned that if we use the term 'shrink' users will think that this option prevents VACUUM FULL from working. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
RE: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Masahiko Sawada [mailto:sawada.m...@gmail.com] > "VACUUM" needs or "vacuum" is more appropriate here? Looking at the same file and some other files, "vacuum" looks appropriate because it represents the vacuum action, not the specific VACUUM command. > The format of the documentation of new option is a bit weird. Could it > be possible to adjust it around 80 characters per line like other > description? Ah, fixed. It's easy to overlook the style with the screen reader software... I've been wondering if there are good settings for editing .sgml in Emacs that, for example, puts appropriate number of spaces at the beginning of each line when is pressed, automatically break the long line and put spaces, etc. From: Julien Rouhaud [mailto:rjuju...@gmail.com] > also, the documentation should point out that freeing is not > guaranteed. Something like > > + The default is true. If true, VACUUM will try to free empty > pages at the end of the table. That's nice. Done. > > I'm not sure the consensus we got here but we don't make the vacuum > > command option for this? > > I don't think here's a clear consensus, but my personal vote is to add > it, with SHRINK_TABLE = [ force_on | force_off | default ] (unless a > better proposal has been made already) IMO, which I mentioned earlier, I don't think the VACUUM option is necessary because: (1) this is a table property which is determined based on the expected workload, not the one that people want to change per VACUUM operation (2) if someone wants to change the behavior for a particular VACUUM operation, he can do it using ALTER TABLE SET. Anyway, we can add the VACUUM option separately if we want it by all means. I don't it to be the blocker for this feature to be included in PG 12, because the vacuum truncaton has been bothering us like others said in this and other threads... Regards Takayuki Tsunakawa disable-vacuum-truncation_v6.patch Description: disable-vacuum-truncation_v6.patch
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Thu, Apr 4, 2019 at 1:23 PM Masahiko Sawada wrote: > > On Thu, Apr 4, 2019 at 1:26 PM Tsunakawa, Takayuki > wrote: > > > > From: Fujii Masao [mailto:masao.fu...@gmail.com] > > > reloption for TOAST is also required? > > > > # I've come back to the office earlier than planned... > > > > Hm, there's no reason to not provide toast.vacuum_shrink_enabled. Done > > with the attached patch. > > > > Thank you for updating the patch! +1! > +vacuum_shrink_enabled, > toast.vacuum_shrink_enabled > (boolean) > + > + > + Enables or disables shrinking the table when it's vacuumed. > + This also applies to autovacuum. > + The default is true. If true, VACUUM frees empty pages at the > end of the table. > > "VACUUM" needs or "vacuum" is more appropriate here? also, the documentation should point out that freeing is not guaranteed. Something like + The default is true. If true, VACUUM will try to free empty pages at the end of the table. > I'm not sure the consensus we got here but we don't make the vacuum > command option for this? I don't think here's a clear consensus, but my personal vote is to add it, with SHRINK_TABLE = [ force_on | force_off | default ] (unless a better proposal has been made already)
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Thu, Apr 4, 2019 at 1:26 PM Tsunakawa, Takayuki wrote: > > From: Fujii Masao [mailto:masao.fu...@gmail.com] > > reloption for TOAST is also required? > > # I've come back to the office earlier than planned... > > Hm, there's no reason to not provide toast.vacuum_shrink_enabled. Done with > the attached patch. > Thank you for updating the patch! +vacuum_shrink_enabled, toast.vacuum_shrink_enabled (boolean) + + + Enables or disables shrinking the table when it's vacuumed. + This also applies to autovacuum. + The default is true. If true, VACUUM frees empty pages at the end of the table. "VACUUM" needs or "vacuum" is more appropriate here? + Shrinking the table requires ACCESS EXCLUSIVE lock on the table. + It can take non-negligible time when the shared buffer is large. Besides, ACCESS EXCLUSIVE + lock could lead to query cancellation on the standby server. + If the workload is likely to reuse the freed space soon + (e.g., UPDATE-heavy, or new rows will be added after deleting + old rows), setting this parameter to false makes sense to avoid unnecessary shrinking. + + + The format of the documentation of new option is a bit weird. Could it be possible to adjust it around 80 characters per line like other description? I'm not sure the consensus we got here but we don't make the vacuum command option for this? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
RE: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Fujii Masao [mailto:masao.fu...@gmail.com] > reloption for TOAST is also required? # I've come back to the office earlier than planned... Hm, there's no reason to not provide toast.vacuum_shrink_enabled. Done with the attached patch. Regards Takayuki Tsunakawa disable-vacuum-truncation_v5.patch Description: disable-vacuum-truncation_v5.patch
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Thu, Mar 28, 2019 at 11:45 AM Tsunakawa, Takayuki wrote: > > From: Robert Haas [mailto:robertmh...@gmail.com] > > You're both right and I'm wrong. > > > > However, I think it would be better to stick with the term 'truncate' > > which is widely-used already, rather than introducing a new term. > > Yeah, I have the same feeling. OTOH, as I referred in this thread, shrink is > used instead of truncate in the PostgreSQL documentation. So, I chose > shrink. To repeat myself, I'm comfortable with either word. I'd like the > committer to choose what he thinks better. + This parameter cannot be set for TOAST tables. reloption for TOAST is also required? Regards, -- Fujii Masao
RE: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Robert Haas [mailto:robertmh...@gmail.com] > You're both right and I'm wrong. > > However, I think it would be better to stick with the term 'truncate' > which is widely-used already, rather than introducing a new term. Yeah, I have the same feeling. OTOH, as I referred in this thread, shrink is used instead of truncate in the PostgreSQL documentation. So, I chose shrink. To repeat myself, I'm comfortable with either word. I'd like the committer to choose what he thinks better. Regards Takayuki Tsunakawa
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Tue, Mar 26, 2019 at 10:35 PM Tsunakawa, Takayuki wrote: > I almost have the same view as Sawada-san. The reloption > vacuum_shrink_enabled is a positive name and follows the naming style of > other reloptions. I hope this matches the style you have in mind. You're both right and I'm wrong. However, I think it would be better to stick with the term 'truncate' which is widely-used already, rather than introducing a new term. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
RE: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Masahiko Sawada [mailto:sawada.m...@gmail.com] > On Wed, Mar 27, 2019 at 2:30 AM Robert Haas wrote: > > > > On Tue, Mar 26, 2019 at 11:23 AM Masahiko Sawada > wrote: > > > > I don't see a patch with the naming updated, here or there, and I'm > > > > going to be really unhappy if we end up with inconsistent naming > > > > between two patches that do such fundamentally similar things. -1 > > > > from me to committing either one until that inconsistency is resolved. > > > > > > Agreed. I've just submitted the latest version patch that adds > > > INDEX_CLEANUP option and vacuum_index_cleanup reloption. I already > > > mentioned on that thread but I agreed with adding phrase positively > > > than negatively. So if we got consensus on such naming the new options > > > added by this patch could be something like SHRINK option (with > > > true/false) and vacuum_shrink reloption. > > > > No, that's just perpetuating the problem. Then you have an option > > SHRINK here that you set to TRUE to skip something, and an option > > INDEX_CLEANUP over there that you set to FALSE to skip something. > > > > Well, I imagined that both INDEX_CLEANUP option and SHRINK option (or > perhaps TRUNCATE option) should be true by default. If we want to skip > some operation of vacuum we can set each options to false like "VACUUM > (INDEX_CLEANUP false, SHRINK true, VERBOSE true)". I think that > resolves the problem but am I missing something? I almost have the same view as Sawada-san. The reloption vacuum_shrink_enabled is a positive name and follows the naming style of other reloptions. I hope this matches the style you have in mind. Regards Takayuki Tsunakawa
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Wed, Mar 27, 2019 at 2:30 AM Robert Haas wrote: > > On Tue, Mar 26, 2019 at 11:23 AM Masahiko Sawada > wrote: > > > I don't see a patch with the naming updated, here or there, and I'm > > > going to be really unhappy if we end up with inconsistent naming > > > between two patches that do such fundamentally similar things. -1 > > > from me to committing either one until that inconsistency is resolved. > > > > Agreed. I've just submitted the latest version patch that adds > > INDEX_CLEANUP option and vacuum_index_cleanup reloption. I already > > mentioned on that thread but I agreed with adding phrase positively > > than negatively. So if we got consensus on such naming the new options > > added by this patch could be something like SHRINK option (with > > true/false) and vacuum_shrink reloption. > > No, that's just perpetuating the problem. Then you have an option > SHRINK here that you set to TRUE to skip something, and an option > INDEX_CLEANUP over there that you set to FALSE to skip something. > Well, I imagined that both INDEX_CLEANUP option and SHRINK option (or perhaps TRUNCATE option) should be true by default. If we want to skip some operation of vacuum we can set each options to false like "VACUUM (INDEX_CLEANUP false, SHRINK true, VERBOSE true)". I think that resolves the problem but am I missing something? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Tue, Mar 26, 2019 at 11:23 AM Masahiko Sawada wrote: > > I don't see a patch with the naming updated, here or there, and I'm > > going to be really unhappy if we end up with inconsistent naming > > between two patches that do such fundamentally similar things. -1 > > from me to committing either one until that inconsistency is resolved. > > Agreed. I've just submitted the latest version patch that adds > INDEX_CLEANUP option and vacuum_index_cleanup reloption. I already > mentioned on that thread but I agreed with adding phrase positively > than negatively. So if we got consensus on such naming the new options > added by this patch could be something like SHRINK option (with > true/false) and vacuum_shrink reloption. No, that's just perpetuating the problem. Then you have an option SHRINK here that you set to TRUE to skip something, and an option INDEX_CLEANUP over there that you set to FALSE to skip something. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Tue, Mar 26, 2019 at 10:30 PM Robert Haas wrote: > > On Tue, Mar 26, 2019 at 3:57 AM Tsunakawa, Takayuki > wrote: > > From: David Steele [mailto:da...@pgmasters.net] > > > This patch appears to have been stalled for a while. > > > > > > Takayuki -- the ball appears to be in your court. Perhaps it would be > > > helpful to summarize what you think are next steps? > > > > disable_index_cleanup is handled by Sawada-san in another thread. I > > understand I've reflected all review comments in the latest patch, and > > replied to the opinions/proposals, so the patch status is kept "needs > > review." (I hope new fire won't happen...) > > I don't see a patch with the naming updated, here or there, and I'm > going to be really unhappy if we end up with inconsistent naming > between two patches that do such fundamentally similar things. -1 > from me to committing either one until that inconsistency is resolved. Agreed. I've just submitted the latest version patch that adds INDEX_CLEANUP option and vacuum_index_cleanup reloption. I already mentioned on that thread but I agreed with adding phrase positively than negatively. So if we got consensus on such naming the new options added by this patch could be something like SHRINK option (with true/false) and vacuum_shrink reloption. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Tue, Mar 26, 2019 at 3:57 AM Tsunakawa, Takayuki wrote: > From: David Steele [mailto:da...@pgmasters.net] > > This patch appears to have been stalled for a while. > > > > Takayuki -- the ball appears to be in your court. Perhaps it would be > > helpful to summarize what you think are next steps? > > disable_index_cleanup is handled by Sawada-san in another thread. I > understand I've reflected all review comments in the latest patch, and > replied to the opinions/proposals, so the patch status is kept "needs > review." (I hope new fire won't happen...) I don't see a patch with the naming updated, here or there, and I'm going to be really unhappy if we end up with inconsistent naming between two patches that do such fundamentally similar things. -1 from me to committing either one until that inconsistency is resolved. I have made a proposal for resolving it in a way that I think would be satisfactory and best; other options might also exist; the patch looks unproblematic otherwise; but I don't think it is committable as it stands. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
RE: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: David Steele [mailto:da...@pgmasters.net] > This patch appears to have been stalled for a while. > > Takayuki -- the ball appears to be in your court. Perhaps it would be > helpful to summarize what you think are next steps? disable_index_cleanup is handled by Sawada-san in another thread. I understand I've reflected all review comments in the latest patch, and replied to the opinions/proposals, so the patch status is kept "needs review." (I hope new fire won't happen...) Regards Takayuki Tsunakawa
Re: Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 3/5/19 6:41 AM, Masahiko Sawada wrote: I understood the use case. I'm inclined to add DISABLE_INDEX_CLEANUP as a reloption. It's an improvement but it seems to me that the specifying a threshold or scale factor would be more useful for that case than just turning on and off. It's something like autovaucum_index_vacuum_scale_factor, 0 by default means always trigger index vacuuming and -1 means never trigger. This patch appears to have been stalled for a while. Takayuki -- the ball appears to be in your court. Perhaps it would be helpful to summarize what you think are next steps? Regards, -- -David da...@pgmasters.net
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Tue, Mar 5, 2019 at 5:11 AM Andres Freund wrote: > > Hi, > > On 2019-03-04 20:03:37 +, Bossart, Nathan wrote: > > On 3/3/19, 9:23 PM, "Masahiko Sawada" wrote: > > > FWIW, I agree that we have options for vacuum as vacuum > > > command options. But for reloptions, I think if the persistence the > > > setting could be problematic we should not. According to the > > > discussions so far, I think VACUUM_SHRINK_ENABLED is the one option > > > that can be available as both vacuum command option and reloptions. > > > But I'm not sure there is good use case even if we can set > > > DISABLE_INDEX_CLEANUP as reloptions. > > > > +1 > > > > The DISABLE_INDEX_CLEANUP option is intended to help avoid transaction > > ID wraparound and should not be used as a long-term VACUUM strategy > > for a table. > > I'm not quite convinced this is right. There's plenty sites that > practically can't use autovacuum because it might decide to vacuum the > 5TB index because of 300 dead tuples in the middle of busy periods. And > without an reloption that's not controllable. I understood the use case. I'm inclined to add DISABLE_INDEX_CLEANUP as a reloption. It's an improvement but it seems to me that the specifying a threshold or scale factor would be more useful for that case than just turning on and off. It's something like autovaucum_index_vacuum_scale_factor, 0 by default means always trigger index vacuuming and -1 means never trigger. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 3/4/19, 2:05 PM, "Andres Freund" wrote: > On 2019-03-04 22:00:47 +, Bossart, Nathan wrote: >> On 3/4/19, 1:44 PM, "Andres Freund" wrote: >> > Yea, I do think that's a danger. But we allow disabling autovacuum, so >> > I'm not sure it matters that much... And for indexes you'd still have >> > the index page-level vacuum that'd continue to work. >> >> I think the difference here is that there isn't something like >> autovacuum_freeze_max_age to force index cleanup at some point. >> Granted, you can set autovacuum_freeze_max_age to 2B if you want, but >> at least there's a fallback available. > > Well, but your cluster doesn't suddenly shut down because of index bloat > (in contrast to xid wraparound). So I don't quite see an equivalent need > for an emergency valve. I think we should just put a warning into the > reloption's docs, and leave it at that. That seems reasonable to me. Nathan
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi, On 2019-03-04 22:00:47 +, Bossart, Nathan wrote: > On 3/4/19, 1:44 PM, "Andres Freund" wrote: > > Yea, I do think that's a danger. But we allow disabling autovacuum, so > > I'm not sure it matters that much... And for indexes you'd still have > > the index page-level vacuum that'd continue to work. > > I think the difference here is that there isn't something like > autovacuum_freeze_max_age to force index cleanup at some point. > Granted, you can set autovacuum_freeze_max_age to 2B if you want, but > at least there's a fallback available. Well, but your cluster doesn't suddenly shut down because of index bloat (in contrast to xid wraparound). So I don't quite see an equivalent need for an emergency valve. I think we should just put a warning into the reloption's docs, and leave it at that. Greetings, Andres Freund
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 3/4/19, 1:44 PM, "Andres Freund" wrote: > On 2019-03-04 21:40:53 +, Bossart, Nathan wrote: >> On 3/4/19, 12:11 PM, "Andres Freund" wrote: >> > I'm not quite convinced this is right. There's plenty sites that >> > practically can't use autovacuum because it might decide to vacuum the >> > 5TB index because of 300 dead tuples in the middle of busy periods. And >> > without an reloption that's not controllable. >> >> Wouldn't it be better to adjust the cost and threshold parameters or >> to manually vacuum during quieter periods? > > No. (auto)vacuum is useful to reclaim space etc. It's just the > unnecessary index cleanup that's the problem... Most of the space can > be reclaimed after all, the item pointer ain't that big... I see what you mean. >> I suppose setting DISABLE_INDEX_CLEANUP on a relation during busy >> periods could be useful if you really need to continue reclaiming >> transaction IDs, but that seems like an easy way to accidentally never >> vacuum indexes. > > Yea, I do think that's a danger. But we allow disabling autovacuum, so > I'm not sure it matters that much... And for indexes you'd still have > the index page-level vacuum that'd continue to work. I think the difference here is that there isn't something like autovacuum_freeze_max_age to force index cleanup at some point. Granted, you can set autovacuum_freeze_max_age to 2B if you want, but at least there's a fallback available. Nathan
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi, On 2019-03-04 21:40:53 +, Bossart, Nathan wrote: > On 3/4/19, 12:11 PM, "Andres Freund" wrote: > > I'm not quite convinced this is right. There's plenty sites that > > practically can't use autovacuum because it might decide to vacuum the > > 5TB index because of 300 dead tuples in the middle of busy periods. And > > without an reloption that's not controllable. > > Wouldn't it be better to adjust the cost and threshold parameters or > to manually vacuum during quieter periods? No. (auto)vacuum is useful to reclaim space etc. It's just the unnecessary index cleanup that's the problem... Most of the space can be reclaimed after all, the item pointer ain't that big... > I suppose setting DISABLE_INDEX_CLEANUP on a relation during busy > periods could be useful if you really need to continue reclaiming > transaction IDs, but that seems like an easy way to accidentally never > vacuum indexes. Yea, I do think that's a danger. But we allow disabling autovacuum, so I'm not sure it matters that much... And for indexes you'd still have the index page-level vacuum that'd continue to work. - Andres
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 3/4/19, 12:11 PM, "Andres Freund" wrote: > I'm not quite convinced this is right. There's plenty sites that > practically can't use autovacuum because it might decide to vacuum the > 5TB index because of 300 dead tuples in the middle of busy periods. And > without an reloption that's not controllable. Wouldn't it be better to adjust the cost and threshold parameters or to manually vacuum during quieter periods? I suppose setting DISABLE_INDEX_CLEANUP on a relation during busy periods could be useful if you really need to continue reclaiming transaction IDs, but that seems like an easy way to accidentally never vacuum indexes. Nathan
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi, On 2019-03-04 20:03:37 +, Bossart, Nathan wrote: > On 3/3/19, 9:23 PM, "Masahiko Sawada" wrote: > > FWIW, I agree that we have options for vacuum as vacuum > > command options. But for reloptions, I think if the persistence the > > setting could be problematic we should not. According to the > > discussions so far, I think VACUUM_SHRINK_ENABLED is the one option > > that can be available as both vacuum command option and reloptions. > > But I'm not sure there is good use case even if we can set > > DISABLE_INDEX_CLEANUP as reloptions. > > +1 > > The DISABLE_INDEX_CLEANUP option is intended to help avoid transaction > ID wraparound and should not be used as a long-term VACUUM strategy > for a table. I'm not quite convinced this is right. There's plenty sites that practically can't use autovacuum because it might decide to vacuum the 5TB index because of 300 dead tuples in the middle of busy periods. And without an reloption that's not controllable. - Andres
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 3/3/19, 9:23 PM, "Masahiko Sawada" wrote: > FWIW, I agree that we have options for vacuum as vacuum > command options. But for reloptions, I think if the persistence the > setting could be problematic we should not. According to the > discussions so far, I think VACUUM_SHRINK_ENABLED is the one option > that can be available as both vacuum command option and reloptions. > But I'm not sure there is good use case even if we can set > DISABLE_INDEX_CLEANUP as reloptions. +1 The DISABLE_INDEX_CLEANUP option is intended to help avoid transaction ID wraparound and should not be used as a long-term VACUUM strategy for a table. Nathan
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Sat, Mar 2, 2019 at 4:34 AM Tom Lane wrote: > > Andrew Dunstan writes: > > On 3/1/19 2:14 PM, Tom Lane wrote: > >> Indeed, but I'm not sure that the use-cases are the same. In particular, > >> unless somebody has done some rather impossible magic, it would be > >> disastrous to apply DISABLE_INDEX_CLEANUP as a reloption, because then > >> it would be persistent and you'd never get a real vacuum operation and > >> soon your disk would be full. Permanently applying truncation disabling > >> seems less insane. > > > You could allow an explicitly set command option to override the reloption. > > It's important for us to be able to control the vacuum phases more. In > > particular, the index cleanup phase can have significant system impact > > but often doesn't need to be done immediately. > > I'm not objecting to having a manual command option to skip index cleanup > (which basically reduces to "do nothing but tuple freezing", right? DISABLE_INDEX_CLEANUP option does freezing tuples, HOT-pruning and mark tuples as dead but skips removing tuples, index vacuuming and index cleanup. > maybe it should be named/documented that way). Applying it as a reloption > seems like a foot-gun, though. FWIW, I agree that we have options for vacuum as vacuum command options. But for reloptions, I think if the persistence the setting could be problematic we should not. According to the discussions so far, I think VACUUM_SHRINK_ENABLED is the one option that can be available as both vacuum command option and reloptions. But I'm not sure there is good use case even if we can set DISABLE_INDEX_CLEANUP as reloptions. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Andrew Dunstan writes: > On 3/1/19 2:14 PM, Tom Lane wrote: >> Indeed, but I'm not sure that the use-cases are the same. In particular, >> unless somebody has done some rather impossible magic, it would be >> disastrous to apply DISABLE_INDEX_CLEANUP as a reloption, because then >> it would be persistent and you'd never get a real vacuum operation and >> soon your disk would be full. Permanently applying truncation disabling >> seems less insane. > You could allow an explicitly set command option to override the reloption. > It's important for us to be able to control the vacuum phases more. In > particular, the index cleanup phase can have significant system impact > but often doesn't need to be done immediately. I'm not objecting to having a manual command option to skip index cleanup (which basically reduces to "do nothing but tuple freezing", right? maybe it should be named/documented that way). Applying it as a reloption seems like a foot-gun, though. regards, tom lane
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Andres Freund writes: > On 2019-03-01 14:17:33 -0500, Tom Lane wrote: >> I think we should reject the whole patch, tbh, and go do something >> about the underlying problem instead. Once we've made truncation >> not require AEL, this will be nothing but a legacy wart that we'll >> have a hard time getting rid of. > IDK, it's really painful in the field, and I'm not quite seeing us > getting rid of the AEL for v12. Dunno, I was musing about it just yesterday, in https://postgr.es/m/1261.1551392...@sss.pgh.pa.us I'd sure rather spend time making that happen than this. I'm also not entirely convinced that we MUST do something about this in v12 rather than v13 --- we've been living with it ever since we had in-core replication, why's it suddenly so critical? > I think it's a wart, but one that works > around a pretty important usability issue. And I think we should just > remove the GUC without any sort of deprecation afterwards, if necessary > we can add a note to the docs to that effect. It's not like preventing > truncation from happening is a very intrusive / dangerous thing to do. Well, if we add a reloption then we can never ever get rid of it; at best we could ignore it. So from the perspective of how-fast-can-we- deprecate-this, maybe a GUC is the better answer. On the other hand, I'm not sure I believe that many installations could afford to disable truncation for every single table. regards, tom lane
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 3/1/19 2:14 PM, Tom Lane wrote: > Robert Haas writes: >> I want to make one other point about this patch, which is that over on >> the thread "New vacuum option to do only freezing" we have a patch >> that does a closely-related thing. Both patches skip one phase of the >> overall VACUUM process. THIS patch wants to skip truncation; THAT >> patch wants to skip index cleanup. Over there, we seem to have >> settled on DISABLE_INDEX_CLEANUP -- only available as a VACUUM option >> -- and here I think the proposal is currently VACUUM_SHRINK_ENABLED -- >> only available as a reloption. >> Now that seems not very consistent. > Indeed, but I'm not sure that the use-cases are the same. In particular, > unless somebody has done some rather impossible magic, it would be > disastrous to apply DISABLE_INDEX_CLEANUP as a reloption, because then > it would be persistent and you'd never get a real vacuum operation and > soon your disk would be full. Permanently applying truncation disabling > seems less insane. > > The gratuitously inconsistent spellings should be harmonized, for sure. > > You could allow an explicitly set command option to override the reloption. It's important for us to be able to control the vacuum phases more. In particular, the index cleanup phase can have significant system impact but often doesn't need to be done immediately. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi, On 2019-03-01 14:17:33 -0500, Tom Lane wrote: > Andres Freund writes: > > OTOH, as the main reason for wanting to disable truncation is that a > > user is getting very undesirable HS conflicts, it doesn't seem right to > > force them to change the reloption on all tables, and then somehow force > > it to be set on all tables created at a later stage. I'm not sure how > > that'd be better? > > I think we should reject the whole patch, tbh, and go do something > about the underlying problem instead. Once we've made truncation > not require AEL, this will be nothing but a legacy wart that we'll > have a hard time getting rid of. IDK, it's really painful in the field, and I'm not quite seeing us getting rid of the AEL for v12. I think it's a wart, but one that works around a pretty important usability issue. And I think we should just remove the GUC without any sort of deprecation afterwards, if necessary we can add a note to the docs to that effect. It's not like preventing truncation from happening is a very intrusive / dangerous thing to do. Greetings, Andres Freund
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Andres Freund writes: > OTOH, as the main reason for wanting to disable truncation is that a > user is getting very undesirable HS conflicts, it doesn't seem right to > force them to change the reloption on all tables, and then somehow force > it to be set on all tables created at a later stage. I'm not sure how > that'd be better? I think we should reject the whole patch, tbh, and go do something about the underlying problem instead. Once we've made truncation not require AEL, this will be nothing but a legacy wart that we'll have a hard time getting rid of. regards, tom lane
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Robert Haas writes: > I want to make one other point about this patch, which is that over on > the thread "New vacuum option to do only freezing" we have a patch > that does a closely-related thing. Both patches skip one phase of the > overall VACUUM process. THIS patch wants to skip truncation; THAT > patch wants to skip index cleanup. Over there, we seem to have > settled on DISABLE_INDEX_CLEANUP -- only available as a VACUUM option > -- and here I think the proposal is currently VACUUM_SHRINK_ENABLED -- > only available as a reloption. > Now that seems not very consistent. Indeed, but I'm not sure that the use-cases are the same. In particular, unless somebody has done some rather impossible magic, it would be disastrous to apply DISABLE_INDEX_CLEANUP as a reloption, because then it would be persistent and you'd never get a real vacuum operation and soon your disk would be full. Permanently applying truncation disabling seems less insane. The gratuitously inconsistent spellings should be harmonized, for sure. regards, tom lane
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi, On 2019-02-27 10:55:49 -0500, Robert Haas wrote: > I don't think that a VACUUM option would be out of place, but a GUC > sounds like an attractive nuisance to me. It will encourage people to > just flip it blindly instead of considering the particular cases where > they need that behavior, and I think chances are good that most people > who do that will end up being sad. OTOH, as the main reason for wanting to disable truncation is that a user is getting very undesirable HS conflicts, it doesn't seem right to force them to change the reloption on all tables, and then somehow force it to be set on all tables created at a later stage. I'm not sure how that'd be better? Greetings, Andres Freund
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 3/1/19 1:43 PM, Robert Haas wrote: > On Thu, Feb 28, 2019 at 3:17 AM Tsunakawa, Takayuki > wrote: >> Uh, thanks. I've just recognized I didn't know the meaning of "nuisance." >> I've looked up the meaning in the dictionary. Nuisance is like a trouble >> maker... > My proposal would be that we make both options available as both > reloptions and vacuum options. +many cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Thu, Feb 28, 2019 at 3:17 AM Tsunakawa, Takayuki wrote: > Uh, thanks. I've just recognized I didn't know the meaning of "nuisance." > I've looked up the meaning in the dictionary. Nuisance is like a trouble > maker... Yes, and "attractive nuisance" means something that, superficially, it looks like a good idea, but later, you find out that it creates many problems. I want to make one other point about this patch, which is that over on the thread "New vacuum option to do only freezing" we have a patch that does a closely-related thing. Both patches skip one phase of the overall VACUUM process. THIS patch wants to skip truncation; THAT patch wants to skip index cleanup. Over there, we seem to have settled on DISABLE_INDEX_CLEANUP -- only available as a VACUUM option -- and here I think the proposal is currently VACUUM_SHRINK_ENABLED -- only available as a reloption. Now that seems not very consistent. One can only be set as a reloption, the other only as a VACUUM option. One talks about what is enabled, the other about what is disabled. One puts enable/disable at the start of the name, the other at the end. My proposal would be that we make both options available as both reloptions and vacuum options. Call the VACUUM options INDEX_CLEANUP and TRUNCATE and the default will be true but the user can specify false. For the reloptions, prefix "vacuum_", thus vacuum_index_cleanup = true/false and vacuum_truncate = true/false. If that doesn't appeal, I am open to other ideas how to make this consistent, but I think it should in some way be made consistent. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Alvaro Herrera writes: > On 2019-Feb-28, Tom Lane wrote: >> I wasn't really working on that for v12 --- I figured it was way >> too late in the cycle to be starting on such a significant change. > Oh, well, it certainly seems far too late *now*. However, what about > the idea in > https://postgr.es/m/1255.1544562...@sss.pgh.pa.us > namely that we write out the buffers involved? That sounds like it > might be backpatchable, and thus it's not too late for it. I think that what we had in mind at that point was that allowing forced writes of empty-but-dirty pages would provide a back-patchable solution to the problem of ftruncate() failure leaving corrupt state on-disk. That would not, by itself, remove the need for AccessExclusiveLock, so it doesn't seem like it would eliminate people's desire for the kind of knob being discussed here. Thinking about it, the need for AEL is mostly independent of the data corruption problem; rather, it's a hack to avoid needing to think about concurrent-truncation scenarios in table readers. We could fairly easily reduce the lock level to something less than AEL if we just taught seqscans, indexscans, etc that trying to read a page beyond EOF is not an error. (Reducing the lock level to the point where we could allow concurrent *writers* is a much harder problem, I think. But to ameliorate the issues for standbys, we just need to allow concurrent readers.) And we'd have to do something about readers possibly loading doomed pages back into shmem before the truncation happens; maybe that can be fixed just by truncating first and flushing buffers second? I think the $64 question is whether we're giving up any meaningful degree of error detection if we allow read-beyond-EOF to not be an error. If we conclude that we're not, maybe it wouldn't be a very big patch? regards, tom lane
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 2019-Feb-28, Tom Lane wrote: > Alvaro Herrera writes: > > Hopefully we'll get Tom's patch that addresses the failure-to-truncate > > issues in pg12. > > Hm, are you speaking of the handwaving I did in > https://www.postgresql.org/message-id/2348.1544474...@sss.pgh.pa.us > ? > > I wasn't really working on that for v12 --- I figured it was way > too late in the cycle to be starting on such a significant change. Oh, well, it certainly seems far too late *now*. However, what about the idea in https://postgr.es/m/1255.1544562...@sss.pgh.pa.us namely that we write out the buffers involved? That sounds like it might be backpatchable, and thus it's not too late for it. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Tsunakawa, Takayuki wrote: > Why do you think that it's better for VACUUM command to have the option? I > think it's a > table property whose value is determined based on the application workload, > not per VACUUM > execution. Rather, I think GUC is more useful to determine the behavior of > the entire > database and/or application. I cannot speak for Alvaro, but I think that many people think that a global setting is too dangerous (I personally don't think so). And if we don't have a GUC, an option to VACUUM would be convenient for one-time clean-up of a table where taking a truncation lock would be too disruptive. > If we want to change a given execution of VACUUM, then we can ALTER TABLE > SET, VACUUM, > and ALTER TABLE SET back. True. That ALTER TABLE would probably need a SHARE UPDATE EXCLUSIVE lock on the table, and that's no worse than VACUUM itself. Yours, Laurenz Albe
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com] > Robert used the phrase "attractive nuisance", which maybe sounds like a > good thing to have to a non native speaker, but it actually isn't -- he > was saying we should avoid a GUC at all, and I can see the reason for > that. I think we should have a VACUUM option and a reloption, but no > GUC. Uh, thanks. I've just recognized I didn't know the meaning of "nuisance." I've looked up the meaning in the dictionary. Nuisance is like a trouble maker... Why do you think that it's better for VACUUM command to have the option? I think it's a table property whose value is determined based on the application workload, not per VACUUM execution. Rather, I think GUC is more useful to determine the behavior of the entire database and/or application. If we want to change a given execution of VACUUM, then we can ALTER TABLE SET, VACUUM, and ALTER TABLE SET back. Regards Takayuki Tsunakawa
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Hi > The default should always be to shrink, unless either the VACUUM > option or the reloption turn that off. (So it doesn't make sense to set > either the VACUUM option or the reloption to "on"). I think VACUUM option can be set to "on" by hand in order to override reloption only for this VACUUM call. regards, Sergei
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Alvaro Herrera wrote: > I think we should have a VACUUM option and a reloption, but no > GUC. The default should always be to shrink, unless either the VACUUM > option or the reloption turn that off. (So it doesn't make sense to set > either the VACUUM option or the reloption to "on"). +1 Yours, Laurenz Albe
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Alvaro Herrera writes: > Hopefully we'll get Tom's patch that addresses the failure-to-truncate > issues in pg12. Hm, are you speaking of the handwaving I did in https://www.postgresql.org/message-id/2348.1544474...@sss.pgh.pa.us ? I wasn't really working on that for v12 --- I figured it was way too late in the cycle to be starting on such a significant change. Still, if we did manage to make that work, it would remove the need for user-visible kluges like the one discussed in this thread. regards, tom lane
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On 2019-Feb-28, Tsunakawa, Takayuki wrote:
> From: Michael Paquier [mailto:mich...@paquier.xyz]
> > So we could you consider adding an option for the VACUUM command as well
> > as vacuumdb? The interactions with the current patch is that you need to
> > define the behavior at the beginning of vacuum for a given heap, instead
> > of reading the parameter at the time the truncation happens, and give
>
> I'm not confident whether this is the same as the above, I imagined this:
>
> * Add a new USERSET GUC vacuum_shrink_table = {on | off}, on by default.
> This follows the naming style "verb_object" like log_connections and
> enable_xxx. We may want to use enable_vacuum_shrink or something like that,
> but enable_xxx seems to be used solely for planner control. Plus,
> vacuum-related parameters seem to start with vacuum_.
Robert used the phrase "attractive nuisance", which maybe sounds like a
good thing to have to a non native speaker, but it actually isn't -- he
was saying we should avoid a GUC at all, and I can see the reason for
that. I think we should have a VACUUM option and a reloption, but no
GUC. The default should always be to shrink, unless either the VACUUM
option or the reloption turn that off. (So it doesn't make sense to set
either the VACUUM option or the reloption to "on").
Disclaimer: I did write roughly the same patch using both a GUC and a
VACUUM option, though I named my GUC truncate_on_vacuum and the VACUUM
option "truncate_anyway" (so you can turn truncation off globally, then
enable it selectively at manual vacuum execution time, but not
autovacuum). However, the reason for doing this were concerns about
robustness caused by data corruption induced by failing to truncate
pages containing removed tuples ... not performance improvement, as your
patch. So they wanted to turn off truncation for *all* tables, not just
a select few.
Hopefully we'll get Tom's patch that addresses the failure-to-truncate
issues in pg12.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Michael Paquier [mailto:mich...@paquier.xyz]
> So we could you consider adding an option for the VACUUM command as well
> as vacuumdb? The interactions with the current patch is that you need to
> define the behavior at the beginning of vacuum for a given heap, instead
> of reading the parameter at the time the truncation happens, and give
I'm not confident whether this is the same as the above, I imagined this:
* Add a new USERSET GUC vacuum_shrink_table = {on | off}, on by default.
This follows the naming style "verb_object" like log_connections and
enable_xxx. We may want to use enable_vacuum_shrink or something like that,
but enable_xxx seems to be used solely for planner control. Plus,
vacuum-related parameters seem to start with vacuum_.
* Give priority to the reloption, because it's targeted at a particular table.
If the reloption is not set, the GUC takes effect.
* As a consequence, the user can change the behavior of VACUUM command by
SETting the GUC in the same session in advance, when the reloption is not set.
If the reloption is set, the user can ALTER TABLE SET, VACUUM, and ALTER TABLE
again to restore the table's setting. But I don't think this use case (change
whether to shrink per VACUUM command execution) is necessary. This is no more
than simply possible.
Regards
Takayuki Tsunakawa
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Thu, Feb 28, 2019 at 01:05:07AM +, Tsunakawa, Takayuki wrote: > From: Robert Haas [mailto:robertmh...@gmail.com] >> I don't think that a VACUUM option would be out of place, but a GUC >> sounds like an attractive nuisance to me. It will encourage people to >> just flip it blindly instead of considering the particular cases where >> they need that behavior, and I think chances are good that most people >> who do that will end up being sad. I won't disagree with you on that. I hear enough about people disappointed that VACUUM does not clean up their garbage enough and that tables are bloated.. And making autovacuum too aggressive is no good either. > Ouch, I sent my previous mail before reading this. I can understand > it may be cumbersome to identify and specify each table, so I tend > to agree the parameter in postgresql, which is USERSET to allow > ALTER DATABASE/USER SET to tune specific databases and applications. > But should the vacuuming of system catalogs also follow this > setting? So we could you consider adding an option for the VACUUM command as well as vacuumdb? The interactions with the current patch is that you need to define the behavior at the beginning of vacuum for a given heap, instead of reading the parameter at the time the truncation happens, and give priority to the command-level option. -- Michael signature.asc Description: PGP signature
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Robert Haas [mailto:robertmh...@gmail.com] > I don't think that a VACUUM option would be out of place, but a GUC > sounds like an attractive nuisance to me. It will encourage people to > just flip it blindly instead of considering the particular cases where > they need that behavior, and I think chances are good that most people > who do that will end up being sad. Ouch, I sent my previous mail before reading this. I can understand it may be cumbersome to identify and specify each table, so I tend to agree the parameter in postgresql, which is USERSET to allow ALTER DATABASE/USER SET to tune specific databases and applications. But should the vacuuming of system catalogs also follow this setting? Regards Takayuki Tsunakawa
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Michael Paquier [mailto:mich...@paquier.xyz] > This makes the test page-size sensitive. While we don't ensure that tests > can be run with different page sizes, we should make a maximum effort to > keep the tests compatible if that's easy enough. In this case you could > just use > 0 as base comparison. I can fix that by myself, so no need to > send a new version. Good idea. Done. > Should we also document that the parameter is effective for autovacuum? > The name can lead to confusion regarding that. I'm not sure for the need because autovacuum is just an automatic execution of vacuum, and existing vacuum_xxx parameters also apply to autovacuum. But being specific is good anyway, so I added reference to autovacuum in the description. > Also, shouldn't the relopt check happen in should_attempt_truncation()? > It seems to me that if we use this routine somewhere else then it should > be controlled by the option. That makes sense. Done. > At the same time, we also have REL_TRUNCATE_FRACTION and > REL_TRUNCATE_MINIMUM which could be made equally user-tunnable. > That's more difficult to control, still why don't we also consider this > part? I thought of it, too. But I didn't have a good idea on how to explain those parameters. I'd like to separate it. > Another thing that seems worth thinking about is a system-level GUC, and > an option in the VACUUM command to control if truncation should happen or > not. We have a lot of infrastructure to control such options between vacuum > and autovacuum, so it could be a waste to not consider potential synergies. Being able to specify this parameter in postgresql.conf and SET (especially ALTER DATABASE/USER to target specific databases/applications) might be useful, but I'm not sure... I'm less confident about whether VACUUM command can specify this, because this is a property of table under a specific workload, not a changable property of each VACUUM action. Anyway, I expect it won't be difficult to add those configurability without contradicting the design, so I'm inclined to separate it. From: Masahiko Sawada [mailto:sawada.m...@gmail.com] > Yeah, that would work. Or it's kind of hackie but the rolling back the > insertion instead of INSERT and DELETE might also work. That's good, because it allows us to keep running reloptions test in parallel with other tests. Done. Regards Takayuki Tsunakawa disable-vacuum-truncation_v4.patch Description: disable-vacuum-truncation_v4.patch
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Feb 25, 2019 at 4:25 AM Michael Paquier wrote: > Another thing that seems worth thinking about is a system-level GUC, > and an option in the VACUUM command to control if truncation should > happen or not. We have a lot of infrastructure to control such > options between vacuum and autovacuum, so it could be a waste to not > consider potential synergies. I don't think that a VACUUM option would be out of place, but a GUC sounds like an attractive nuisance to me. It will encourage people to just flip it blindly instead of considering the particular cases where they need that behavior, and I think chances are good that most people who do that will end up being sad. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Tue, Feb 26, 2019 at 3:29 PM Tsunakawa, Takayuki wrote: > > From: Masahiko Sawada [mailto:sawada.m...@gmail.com] > > This test expects that the inserted tuple is always reclaimed by > > subsequent vacuum, but it's not always true if there are concurrent > > transactions. So size of the reloptions_test table will not be 0 if > > the tuple is not vacuumed. In my environment this test sometimes > > failed with 'make check -j 4'. > > Hmm, "make check -j4" certainly fails on my poor VM, too. > > Modifying src/test/regress/parallel_schedule to put the reloptions test on a > separate line seems to have fixed this issue. Do you think this is the > correct remedy? > Yeah, that would work. Or it's kind of hackie but the rolling back the insertion instead of INSERT and DELETE might also work. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Masahiko Sawada [mailto:sawada.m...@gmail.com] > This test expects that the inserted tuple is always reclaimed by > subsequent vacuum, but it's not always true if there are concurrent > transactions. So size of the reloptions_test table will not be 0 if > the tuple is not vacuumed. In my environment this test sometimes > failed with 'make check -j 4'. Hmm, "make check -j4" certainly fails on my poor VM, too. Modifying src/test/regress/parallel_schedule to put the reloptions test on a separate line seems to have fixed this issue. Do you think this is the correct remedy? Regards Takayuki Tsunakawa
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Feb 25, 2019 at 7:01 PM Tsunakawa, Takayuki
wrote:
>
> From: Michael Paquier [mailto:mich...@paquier.xyz]
> On Mon, Feb 25, 2019 at 03:59:21PM +0900, Masahiko Sawada wrote:
> > > Also, I think that this test may fail in case where concurrent
> > > transactions are running. So maybe should not run it in parallel to
> > > other tests.
> >
> > That's why autovacuum is disabled in this specific test, no? A comment
> > may be a good idea.
>
> Exactly. The table is disabled for autovacuum to avoid being influenced by
> autovacuum.
>
This test expects that the inserted tuple is always reclaimed by
subsequent vacuum, but it's not always true if there are concurrent
transactions. So size of the reloptions_test table will not be 0 if
the tuple is not vacuumed. In my environment this test sometimes
failed with 'make check -j 4'.
diff -U3
/home/masahiko/source/postgresql/src/test/regress/expected/reloptions.out
/home/masahiko/source/postgresql/src/test/regress/results/reloptions.out
--- /home/masahiko/source/postgresql/src/test/regress/expected/reloptions.out
2019-02-25 19:10:49.761438066 +0900
+++ /home/masahiko/source/postgresql/src/test/regress/results/reloptions.out
2019-02-25 19:12:34.885437911 +0900
@@ -117,7 +117,7 @@
SELECT pg_relation_size('reloptions_test');
pg_relation_size
--
-0
+ 8192
(1 row)
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Michael Paquier [mailto:mich...@paquier.xyz] On Mon, Feb 25, 2019 at 03:59:21PM +0900, Masahiko Sawada wrote: > > Also, I think that this test may fail in case where concurrent > > transactions are running. So maybe should not run it in parallel to > > other tests. > > That's why autovacuum is disabled in this specific test, no? A comment > may be a good idea. Exactly. The table is disabled for autovacuum to avoid being influenced by autovacuum. Regards Takayuki Tsunakawa
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Feb 25, 2019 at 08:47:28AM +0100, Julien Rouhaud wrote:
> Ah good point. We could also use something like
> pg_relation_size('reloptions_test') /
> current_setting('block_size')::bigint but >0 should be enough for this
> test.
Also, shouldn't the relopt check happen in
should_attempt_truncation()? It seems to me that if we use this
routine somewhere else then it should be controlled by the option.
At the same time, we also have REL_TRUNCATE_FRACTION and
REL_TRUNCATE_MINIMUM which could be made equally user-tunnable.
That's more difficult to control, still why don't we also consider
this part?
Another thing that seems worth thinking about is a system-level GUC,
and an option in the VACUUM command to control if truncation should
happen or not. We have a lot of infrastructure to control such
options between vacuum and autovacuum, so it could be a waste to not
consider potential synergies.
--
Michael
signature.asc
Description: PGP signature
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Feb 25, 2019 at 03:59:21PM +0900, Masahiko Sawada wrote: > Also, I think that this test may fail in case where concurrent > transactions are running. So maybe should not run it in parallel to > other tests. That's why autovacuum is disabled in this specific test, no? A comment may be a good idea. -- Michael signature.asc Description: PGP signature
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Feb 25, 2019 at 7:56 AM Michael Paquier wrote:
>
> On Mon, Feb 25, 2019 at 02:38:05AM +, Tsunakawa, Takayuki wrote:
> > From: Julien Rouhaud [mailto:rjuju...@gmail.com]
> >> One last thing, I think we should at least add one regression test for
> >> this setting. The one you provided previously seems perfectly suited.
> >
> > Thanks, added.
>
> +SELECT pg_relation_size('reloptions_test');
> + pg_relation_size
> +--
> + 8192
> +(1 row)
> This makes the test page-size sensitive. While we don't ensure that
> tests can be run with different page sizes, we should make a maximum
> effort to keep the tests compatible if that's easy enough. In this
> case you could just use > 0 as base comparison. I can fix that by
> myself, so no need to send a new version.
Ah good point. We could also use something like
pg_relation_size('reloptions_test') /
current_setting('block_size')::bigint but >0 should be enough for this
test.
> Should we also document that the parameter is effective for
> autovacuum? The name can lead to confusion regarding that.
+1
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Feb 25, 2019 at 3:56 PM Michael Paquier wrote:
>
> On Mon, Feb 25, 2019 at 02:38:05AM +, Tsunakawa, Takayuki wrote:
> > From: Julien Rouhaud [mailto:rjuju...@gmail.com]
> >> One last thing, I think we should at least add one regression test for
> >> this setting. The one you provided previously seems perfectly suited.
> >
> > Thanks, added.
>
> +SELECT pg_relation_size('reloptions_test');
> + pg_relation_size
> +--
> + 8192
> +(1 row)
> This makes the test page-size sensitive. While we don't ensure that
> tests can be run with different page sizes, we should make a maximum
> effort to keep the tests compatible if that's easy enough.
Also, I think that this test may fail in case where concurrent
transactions are running. So maybe should not run it in parallel to
other tests.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Mon, Feb 25, 2019 at 02:38:05AM +, Tsunakawa, Takayuki wrote:
> From: Julien Rouhaud [mailto:rjuju...@gmail.com]
>> One last thing, I think we should at least add one regression test for
>> this setting. The one you provided previously seems perfectly suited.
>
> Thanks, added.
+SELECT pg_relation_size('reloptions_test');
+ pg_relation_size
+--
+ 8192
+(1 row)
This makes the test page-size sensitive. While we don't ensure that
tests can be run with different page sizes, we should make a maximum
effort to keep the tests compatible if that's easy enough. In this
case you could just use > 0 as base comparison. I can fix that by
myself, so no need to send a new version.
Should we also document that the parameter is effective for
autovacuum? The name can lead to confusion regarding that.
The rest of the patch looks fine to me.
--
Michael
signature.asc
Description: PGP signature
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Michael Paquier [mailto:mich...@paquier.xyz] > I don't think that we want to use a too generic name and it seems more natural > to reflect the context where it is used in the parameter name. > If we were to shrink with a similar option for other contexts, we would > most likely use a different option. Depending on the load pattern, users > should also be able to disable or enable a subset of contexts as well. > > So I agree with Julien that [auto]vacuum_shrink_enabled is more adapted > for this stuff. OK, I renamed it to vacuum_shrink_enabled. From: Julien Rouhaud [mailto:rjuju...@gmail.com] > One last thing, I think we should at least add one regression test for > this setting. The one you provided previously seems perfectly suited. Thanks, added. Regards Takayuki Tsunakawa disable-vacuum-truncation_v3.patch Description: disable-vacuum-truncation_v3.patch
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Fri, Feb 22, 2019 at 3:39 AM Tsunakawa, Takayuki wrote: > > No, changing the parameter acquires ShareUpdaeExclusive lock. I just > imitated the description for n_distinct in the same comment block. The > meaning is that the setting cannot be changed during VACUUM, so in-flight > VACUUM is not affected. Ah I see, thanks! I find this a little bit confusing but if that's already documented like this for other parameters then I guess that's ok. One last thing, I think we should at least add one regression test for this setting. The one you provided previously seems perfectly suited.
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Fri, Feb 22, 2019 at 02:38:56AM +, Tsunakawa, Takayuki wrote: > From: Julien Rouhaud [mailto:rjuju...@gmail.com] >> FWIW, I prefer shrink over truncate, though I'd rather go with >> vacuum_shink_enabled as suggested previously. > > Thanks. I'd like to leave a committer to choose the name. FWIW, I > chose shrink_enabled rather than vacuum_shrink_enabled because this > property may be used in other shrink situations in the future. What > I imagined was that with the zheap, DELETE or some maintenance > operation, not vacuum, may try to shrink the table. I meant this > property to indicate "whether this table shrinks or not" regardless > of the specific operation that can shrink the table. I don't think that we want to use a too generic name and it seems more natural to reflect the context where it is used in the parameter name. If we were to shrink with a similar option for other contexts, we would most likely use a different option. Depending on the load pattern, users should also be able to disable or enable a subset of contexts as well. So I agree with Julien that [auto]vacuum_shrink_enabled is more adapted for this stuff. -- Michael signature.asc Description: PGP signature
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Julien Rouhaud [mailto:rjuju...@gmail.com] > FWIW, I prefer shrink over truncate, though I'd rather go with > vacuum_shink_enabled as suggested previously. Thanks. I'd like to leave a committer to choose the name. FWIW, I chose shrink_enabled rather than vacuum_shrink_enabled because this property may be used in other shrink situations in the future. What I imagined was that with the zheap, DELETE or some maintenance operation, not vacuum, may try to shrink the table. I meant this property to indicate "whether this table shrinks or not" regardless of the specific operation that can shrink the table. > I'm not sure that I get this comment. Since both require a > ShareUpdateExclusiveLock, you can't change the parameter while a > VACUUM is active on that table. Did you wanted to use another lock > mode? No, changing the parameter acquires ShareUpdaeExclusive lock. I just imitated the description for n_distinct in the same comment block. The meaning is that the setting cannot be changed during VACUUM, so in-flight VACUUM is not affected. Regards Takayuki Tsunakawa
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Michael Paquier [mailto:mich...@paquier.xyz] > On Fri, Feb 01, 2019 at 09:11:50AM +0100, Laurenz Albe wrote: > > Perhaps "vacuum_shrink_enabled" would be even better. > > Naming it just vacuum_truncate and autovacuum_truncate (with aliases for > toast and such), looks more natural to me. "shrink" is not a term used > in the code at all to describe this phase of vacuuming, and this option > talks mainly to people who are experts in PostgreSQL internals in my opinion. FYI, it seems that the user sees "shrink" rather than "truncate" in the documentation as below, although these are about VACUUM FULL. https://www.postgresql.org/docs/devel/sql-vacuum.html would like the table to physically shrink to occupy less disk space https://www.postgresql.org/docs/devel/routine-vacuuming.html shrink a table back to its minimum size and return the disk space to the operating system, Anyway, I don't have any favor about naming this, and I hope native English speakers will choose the best name. I won't object to whatever name any committer chooses. Regards Takayuki Tsunakawa
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
On Fri, Feb 01, 2019 at 09:11:50AM +0100, Laurenz Albe wrote: > Jamison, Kirk wrote: >> I wonder if there is a better reloption name for >> shrink_enabled. (truncate_enabled, vacuum_enabled? Hmm. No?) >> On the other hand, shrink_enabled seems to describe well what it's >> supposed to do when vacuuming tables. Besides there's a >> similarly-named autovacuum_enabled option. > > I like "shrink_enabled". > > It may sound weird in the ears of PostgreSQL hackers, but will make > sense to users. > > Perhaps "vacuum_shrink_enabled" would be even better. Naming it just vacuum_truncate and autovacuum_truncate (with aliases for toast and such), looks more natural to me. "shrink" is not a term used in the code at all to describe this phase of vacuuming, and this option talks mainly to people who are experts in PostgreSQL internals in my opinion. -- Michael signature.asc Description: PGP signature
Re: reloption to prevent VACUUM from truncating empty pages at the end of relation
Jamison, Kirk wrote: > On February 1, 2019, Tsunakawa, Takayuki wrote: > > > > As most people seem to agree adding the reloption, here's the patch. > > > It passes make check, and works like this: > > Sorry, I forgot to include the modified header file. Revised patch > > attached. > > I wonder if there is a better reloption name for shrink_enabled. > (truncate_enabled, vacuum_enabled? Hmm. No?) > On the other hand, shrink_enabled seems to describe well what it's supposed > to do when vacuuming tables. > Besides there's a similarly-named autovacuum_enabled option. I like "shrink_enabled". It may sound weird in the ears of PostgreSQL hackers, but will make sense to users. Perhaps "vacuum_shrink_enabled" would be even better. Yours, Laurenz Albe
RE: reloption to prevent VACUUM from truncating empty pages at the end of relation
From: Jamison, Kirk/ジャミソン カーク > I wonder if there is a better reloption name for shrink_enabled. > (truncate_enabled, vacuum_enabled? Hmm. No?) > On the other hand, shrink_enabled seems to describe well what it's supposed > to do when vacuuming tables. > Besides there's a similarly-named autovacuum_enabled option. Yeah, I used vacuum_truncation_enabled at first. But I thought shrink is better because it represents the final effect from the user perspective, while truncation is the system action to reach the desired state. > I think if most seem to agree to have this solution in place > and to review this further and cover what might be missing, > then shall we register this to next CF? I've already done it. Regards Takayuki Tsunakawa
