Thread: reloption to prevent VACUUM from truncating empty pages at the end of relation

Hi,

I'd like to propose to add $SUBJECT for performance improvement.

When VACUUM tries to truncate the trailing empty pages, it scans shared_buffers
to invalidate the pages-to-truncate during holding an AccessExclusive lock on
the relation. So if shared_buffers is huge, other transactions need to wait for
a very long time before accessing to the relation. Which would cause the
response-time spikes, for example, I observed such spikes several times on
the server with shared_buffers = 300GB while running the benchmark.
Therefore, I'm thinking to propose $SUBJECT and enable it to avoid such spikes
for that relation.

Also, first of all, if other transactions need to extend the relation
(i.e., need new pages) as soon as VACUUM truncates the empty pages at the end,
that truncation would not be so helpful for performance. In this case,
the truncation and extension of the relation are unnecessarily repeated,
which would decrease the performance. So, to alleviate this situation,
$SUBJECT is useful, I think.

Thought?

Regards,

-- 
Fujii Masao

Fujii Masao <masao.fujii@gmail.com> writes:
> When VACUUM tries to truncate the trailing empty pages, it scans shared_buffers
> to invalidate the pages-to-truncate during holding an AccessExclusive lock on
> the relation. So if shared_buffers is huge, other transactions need to wait for
> a very long time before accessing to the relation. Which would cause the
> response-time spikes, for example, I observed such spikes several times on
> the server with shared_buffers = 300GB while running the benchmark.
> Therefore, I'm thinking to propose $SUBJECT and enable it to avoid such spikes
> for that relation.

I think that the real problem here is having to do a scan of all of shared
buffers.  VACUUM's not the only thing that has to do that, there's also
e.g. DROP and TRUNCATE.  So rather than a klugy solution that only fixes
VACUUM (and not very well, requiring user intervention and an unpleasant
tradeoff), we ought to look at ways to avoid needing a whole-pool scan to
find the pages belonging to one relation.  In the past we've been able to
skate by without a decent solution for that because shared buffers were
customarily not all that big.  But if we're going to start considering
huge buffer pools to be a case we want to have good performance for,
that's got to change.

            regards, tom lane

On Tue, Apr 17, 2018 at 11:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> So rather than a klugy solution that only fixes
> VACUUM (and not very well, requiring user intervention and an unpleasant
> tradeoff), we ought to look at ways to avoid needing a whole-pool scan to
> find the pages belonging to one relation.  In the past we've been able to
> skate by without a decent solution for that because shared buffers were
> customarily not all that big.  But if we're going to start considering
> huge buffer pools to be a case we want to have good performance for,
> that's got to change.

Andres mentioned that he has prototyped an approach to buffer
management that uses a Radix tree, which is generally assumed to be
the right long-term fix.

-- 
Peter Geoghegan

Tom Lane wrote:
> Fujii Masao <masao.fujii@gmail.com> writes:
> > When VACUUM tries to truncate the trailing empty pages, it scans shared_buffers
> > to invalidate the pages-to-truncate during holding an AccessExclusive lock on
> > the relation. So if shared_buffers is huge, other transactions need to wait for
> > a very long time before accessing to the relation. Which would cause the
> > response-time spikes, for example, I observed such spikes several times on
> > the server with shared_buffers = 300GB while running the benchmark.
> > Therefore, I'm thinking to propose $SUBJECT and enable it to avoid such spikes
> > for that relation.
> 
> I think that the real problem here is having to do a scan of all of shared
> buffers.  VACUUM's not the only thing that has to do that, there's also
> e.g. DROP and TRUNCATE.  So rather than a klugy solution that only fixes
> VACUUM (and not very well, requiring user intervention and an unpleasant
> tradeoff), we ought to look at ways to avoid needing a whole-pool scan to
> find the pages belonging to one relation.  In the past we've been able to
> skate by without a decent solution for that because shared buffers were
> customarily not all that big.  But if we're going to start considering
> huge buffer pools to be a case we want to have good performance for,
> that's got to change.

Andres was working on a radix tree structure to fix this problem, but
that seems to be abandoned now, and it seems a major undertaking.  While
I agree that the proposed solution is a wart, it seems much better than
no solution at all.  Can we consider Fujii's proposal as a temporary
measure until we fix shared buffers?  I'm +1 on it myself.

We've seen this problem also affecting a production workload pretty
severely, though shared_buffers is not as big.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Andres was working on a radix tree structure to fix this problem, but
> that seems to be abandoned now, and it seems a major undertaking.  While
> I agree that the proposed solution is a wart, it seems much better than
> no solution at all.  Can we consider Fujii's proposal as a temporary
> measure until we fix shared buffers?  I'm +1 on it myself.

Once we've introduced a user-visible reloption it's going to be
practically impossible to get rid of it, so I'm -1.  I'd much rather
see somebody put some effort into the radix-tree idea than introduce
a kluge that we'll be stuck with, and that doesn't even provide a
good user experience.  Disabling vacuum truncation is *not* something
that I think we should recommend.

            regards, tom lane

On 2018-04-17 15:09:18 -0400, Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > Andres was working on a radix tree structure to fix this problem, but
> > that seems to be abandoned now, and it seems a major undertaking.

I hope to re-ignite work on that later in the v12 cycle. But
realistically that means it's not going to be mergable for v12.


> > While I agree that the proposed solution is a wart, it seems much
> > better than no solution at all.  Can we consider Fujii's proposal as
> > a temporary measure until we fix shared buffers?  I'm +1 on it
> > myself.
> 
> Once we've introduced a user-visible reloption it's going to be
> practically impossible to get rid of it, so I'm -1.

It's not much work to maintain though? And even the brief AEL lock can
cause troubles, leaving the scan aside. So I'm like +0.1 or such.

Greetings,

Andres Freund

On Tue, Apr 17, 2018 at 12:12:26PM -0700, Andres Freund wrote:
> On 2018-04-17 15:09:18 -0400, Tom Lane wrote:
>> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>>> Andres was working on a radix tree structure to fix this problem, but
>>> that seems to be abandoned now, and it seems a major undertaking.
>
> I hope to re-ignite work on that later in the v12 cycle. But
> realistically that means it's not going to be mergable for v12.

Need a push of man-hours for that?

>>> While I agree that the proposed solution is a wart, it seems much
>>> better than no solution at all.  Can we consider Fujii's proposal as
>>> a temporary measure until we fix shared buffers?  I'm +1 on it
>>> myself.
>>
>> Once we've introduced a user-visible reloption it's going to be
>> practically impossible to get rid of it, so I'm -1.
>
> It's not much work to maintain though? And even the brief AEL lock can
> cause troubles, leaving the scan aside. So I'm like +0.1 or such.

I would say that if the radix tree patch can make it for the first
commit fest and has reviews, then there would be likely no need for this
reloption.
--
Michael

On April 17, 2018 6:00:59 PM PDT, Michael Paquier <michael@paquier.xyz> wrote:
>On Tue, Apr 17, 2018 at 12:12:26PM -0700, Andres Freund wrote:
>> On 2018-04-17 15:09:18 -0400, Tom Lane wrote:
>>> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>>>> Andres was working on a radix tree structure to fix this problem,
>but
>>>> that seems to be abandoned now, and it seems a major undertaking.
>>
>> I hope to re-ignite work on that later in the v12 cycle. But
>> realistically that means it's not going to be mergable for v12.
>
>Need a push of man-hours for that?

Not sure what you mean?


>>>> While I agree that the proposed solution is a wart, it seems much
>>>> better than no solution at all.  Can we consider Fujii's proposal
>as
>>>> a temporary measure until we fix shared buffers?  I'm +1 on it
>>>> myself.
>>>
>>> Once we've introduced a user-visible reloption it's going to be
>>> practically impossible to get rid of it, so I'm -1.
>>
>> It's not much work to maintain though? And even the brief AEL lock
>can
>> cause troubles, leaving the scan aside. So I'm like +0.1 or such.
>
>I would say that if the radix tree patch can make it for the first
>commit fest and has reviews, then there would be likely no need for
>this
>reloption.

There's no way it can.


Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

On Tue, Apr 17, 2018 at 06:13:31PM -0700, Andres Freund wrote:
> Not sure what you mean?

Do you need help on it?  I suggest that I could undertake the proposed
patch and submit it earlier in the development cycle of v12.
--
Michael

On 2018-04-18 10:46:51 +0900, Michael Paquier wrote:
> On Tue, Apr 17, 2018 at 06:13:31PM -0700, Andres Freund wrote:
> > Not sure what you mean?
> 
> Do you need help on it?  I suggest that I could undertake the proposed
> patch and submit it earlier in the development cycle of v12.

I think it's at the very least two months of serious development work to
get it into a state ready for submission. And a good chunk of that not
even sketched out.  Replacing the hashtable is the easy part, the memory
management (Complicated due to lock-freeness. I'm thinking of using a
variant of epoch based reclamation) isn't really there, the management
of shared "open relations" state are the hard parts...

So yes, I could use help on it, but it'll be a lot of actual design and
investigatory work.

Greetings,

Andres Freund

On Wed, Apr 18, 2018 at 7:46 AM, Andres Freund <andres@anarazel.de> wrote:
> On 2018-04-18 10:46:51 +0900, Michael Paquier wrote:
>> On Tue, Apr 17, 2018 at 06:13:31PM -0700, Andres Freund wrote:
>> > Not sure what you mean?
>>
>> Do you need help on it?  I suggest that I could undertake the proposed
>> patch and submit it earlier in the development cycle of v12.
>
> I think it's at the very least two months of serious development work to
> get it into a state ready for submission. And a good chunk of that not
> even sketched out.  Replacing the hashtable is the easy part, the memory
> management (Complicated due to lock-freeness. I'm thinking of using a
> variant of epoch based reclamation) isn't really there, the management
> of shared "open relations" state are the hard parts...
>
> So yes, I could use help on it, but it'll be a lot of actual design and
> investigatory work.
>

I think it makes sense to pursue that approach, but it might be worth
considering some alternative till we have it.  I remember last time
(in 2015) we have discussed some another solution [1] to this problem
(or similar) and we have left it unattended in the hope that we will
get a better solution, but we are still in the same situation.  I
think in general it is better to go with the approach which can fix
the root cause of the problem, but if that is going to take a long
time, it is not terrible to provide some workable solution which can
help users.


[1] - https://www.postgresql.org/message-id/CAA4eK1JPLGjpMeJ5YLNE7bpNBhP2EQe_rDR%2BAw3atNfj9WkAGg%40mail.gmail.com

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On 17 April 2018 at 20:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> Andres was working on a radix tree structure to fix this problem, but
>> that seems to be abandoned now, and it seems a major undertaking.  While
>> I agree that the proposed solution is a wart, it seems much better than
>> no solution at all.  Can we consider Fujii's proposal as a temporary
>> measure until we fix shared buffers?  I'm +1 on it myself.
>
> Once we've introduced a user-visible reloption it's going to be
> practically impossible to get rid of it, so I'm -1.  I'd much rather
> see somebody put some effort into the radix-tree idea than introduce
> a kluge that we'll be stuck with, and that doesn't even provide a
> good user experience.  Disabling vacuum truncation is *not* something
> that I think we should recommend.

The truncation at the end of VACUUM takes an AccessExclusiveLock,
which is already user visible. Using a radix tree won't alter that.

ISTM the user might be interested in having the *lock* NOT happen, so
I am +1 for the suggestion regardless of whether radix tree ever
happens.

The lock itself can be cancelled, so the user would also be interested
in explicitly requesting a retry with a separate command/function.

-- 
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, Apr 18, 2018 at 11:29 PM, Pavan Deolasee
<pavan.deolasee@gmail.com> wrote:
>
>
> On Tue, Apr 17, 2018 at 11:05 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>> Hi,
>>
>> I'd like to propose to add $SUBJECT for performance improvement.
>>
>> When VACUUM tries to truncate the trailing empty pages, it scans
>> shared_buffers
>> to invalidate the pages-to-truncate during holding an AccessExclusive lock
>> on
>> the relation. So if shared_buffers is huge, other transactions need to
>> wait for
>> a very long time before accessing to the relation. Which would cause the
>> response-time spikes, for example, I observed such spikes several times on
>> the server with shared_buffers = 300GB while running the benchmark.
>> Therefore, I'm thinking to propose $SUBJECT and enable it to avoid such
>> spikes
>> for that relation.
>
>
> Alvaro reminded me that we already have a mechanism in place which forces
> VACUUM to give up the exclusive lock if another backend is waiting on the
> lock for more than certain pre-defined duration. AFAICS we give up the lock,
> but again retry truncation from the previously left off position. What if we
> make that lock-wait duration configurable on a per-table basis? And may be a
> special value to never truncate (though it seems quite excessive to me and a
> possible footgun)

I'm not sure if it's safe to cancel forcibly VACUUM's truncation during
scaning shared_buffers. That scan happens after WAL-logging and before
the actual truncation.

> I was actually thinking in the other direction. So between the time VACUUM
> figures out it can possibly truncate last K pages, some backend may insert a
> tuple in some page and make the truncation impossible. What if we truncate
> the FSM before starting the backward scan so that new inserts go into the
> pages prior to the truncation point, if possible. That will increase the
> chances of VACUUM being able to truncate all the empty pages. Though I think
> in some cases it might lead to unnecessary further extension of the
> relation. May be we use some heuristic based on available free space in the
> table prior to the truncation point?

Isn't this too complicated? I wonder what heuristic we can use here.

Regards,

-- 
Fujii Masao

Pavan Deolasee <pavan.deolasee@gmail.com> writes:
> What if we remember the buffers as seen by count_nondeletable_pages() and
> then just discard those specific buffers instead of scanning the entire
> shared_buffers again?

That's an idea.

> Surely we revisit all to-be-truncated blocks before
> actual truncation. So we already know which buffers to discard. And we're
> holding exclusive lock at that point, so nothing can change underneath. Of
> course, we can't really remember a large number of buffers, so we can do
> this in small chunks.

Hm?  We're deleting the last N consecutive blocks, so it seems like we
just need to think in terms of clearing that range.  I think this can
just be a local logic change inside DropRelFileNodeBuffers().

You could optimize it fairly easily with some heuristic that compares
N to sizeof shared buffers; if it's too large a fraction, the existing
implementation will be cheaper than a bunch of hashtable probes.

            regards, tom lane

I wrote:
> Pavan Deolasee <pavan.deolasee@gmail.com> writes:
>> What if we remember the buffers as seen by count_nondeletable_pages() and
>> then just discard those specific buffers instead of scanning the entire
>> shared_buffers again?

> That's an idea.

BTW, before pushing too hard on any of this, we need to think about the
data-corruption hazard that MauMau just reminded us about.  I'm afraid
what we're likely to end up with after the dust settles is worse
performance than today, not better :-(.

https://postgr.es/m/5BBC590AE8DF4ED1A170E4D48F1B53AC@tunaPC

            regards, tom lane

On Wed, Apr 18, 2018 at 07:41:44PM +0530, Amit Kapila wrote:
> I think it makes sense to pursue that approach, but it might be worth
> considering some alternative till we have it.  I remember last time
> (in 2015) we have discussed some another solution [1] to this problem
> (or similar) and we have left it unattended in the hope that we will
> get a better solution, but we are still in the same situation.  I
> think in general it is better to go with the approach which can fix
> the root cause of the problem, but if that is going to take a long
> time, it is not terrible to provide some workable solution which can
> help users.

Yeah, I can understand that feeling.  When we talked about the
compression of FPWs back in 9.5, we discussed that if we had
double-writes then this would not be necessary, and we are still with
wal_compression but without double writes (actually, it happens that
compression of pages can also be used with double writes, but that's
enough highjacking for this thread..).

Then, let's consider the beginning of the first commit fest of v12 as
judgement.  Implementing radix tree for shared buffers is a long-term
project, which has no guarantee to get merged, while a visibly-simple
reloptions which helps in some cases...
--
Michael

From: Fujii Masao [mailto:masao.fujii@gmail.com]
> a very long time before accessing to the relation. Which would cause the
> response-time spikes, for example, I observed such spikes several times
> on
> the server with shared_buffers = 300GB while running the benchmark.

FYI, a long transaction took about 900 ms, while the average transaction response time was 150 ms or so.  (I'm working
withFujii-san in this performance benchmark.)
 


> Therefore, I'm thinking to propose $SUBJECT and enable it to avoid such
> spikes
> for that relation.

How about an integer variable to replace the following?

#define REL_TRUNCATE_FRACTION    16


> Also, first of all, if other transactions need to extend the relation
> (i.e., need new pages) as soon as VACUUM truncates the empty pages at the
> end,
> that truncation would not be so helpful for performance. In this case,
> the truncation and extension of the relation are unnecessarily repeated,
> which would decrease the performance. So, to alleviate this situation,
> $SUBJECT is useful, I think.

I wonder if fillfactor=50 would alleviate this situation.

Regards
Takayuki Tsunakawa

Michael Paquier wrote:

> Then, let's consider the beginning of the first commit fest of v12 as
> judgement.  Implementing radix tree for shared buffers is a long-term
> project, which has no guarantee to get merged, while a visibly-simple
> reloptions which helps in some cases...

In the scenario we studied, the truncations were causing periodic
hiccups which were quite severe.  The truncations were completely
useless anyway because the table grew back to the original size daily (a
few dozen GBs I think).  That was a lot of unnecessary work, and under
exclusive lock no less.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2018-04-19 16:56:59 -0300, Alvaro Herrera wrote:
> Michael Paquier wrote:
> 
> > Then, let's consider the beginning of the first commit fest of v12 as
> > judgement.  Implementing radix tree for shared buffers is a long-term
> > project, which has no guarantee to get merged, while a visibly-simple
> > reloptions which helps in some cases...
> 
> In the scenario we studied, the truncations were causing periodic
> hiccups which were quite severe.

Was that with the current logic of breaking the truncations into smaller
chunks?


> The truncations were completely
> useless anyway because the table grew back to the original size daily (a
> few dozen GBs I think).  That was a lot of unnecessary work, and under
> exclusive lock no less.

FWIW, One goal of the different buffer mapping implementation is to also
make both increasing and decreasing size of relations possible without
an AEL.

Greetings,

Andres Freund

Andres Freund wrote:
> On 2018-04-19 16:56:59 -0300, Alvaro Herrera wrote:
> > Michael Paquier wrote:
> > 
> > > Then, let's consider the beginning of the first commit fest of v12 as
> > > judgement.  Implementing radix tree for shared buffers is a long-term
> > > project, which has no guarantee to get merged, while a visibly-simple
> > > reloptions which helps in some cases...
> > 
> > In the scenario we studied, the truncations were causing periodic
> > hiccups which were quite severe.
> 
> Was that with the current logic of breaking the truncations into smaller
> chunks?

Yes -- it was with 9.5.7.  I was skeptical about that stuff working
correctly for a toast table, BTW, but I didn't manage to prove anything.

> > The truncations were completely useless anyway because the table
> > grew back to the original size daily (a few dozen GBs I think).
> > That was a lot of unnecessary work, and under exclusive lock no
> > less.
> 
> FWIW, One goal of the different buffer mapping implementation is to also
> make both increasing and decreasing size of relations possible without
> an AEL.

Oh, that sounds very useful.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, 2018-04-17 at 15:09 -0400, Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > Andres was working on a radix tree structure to fix this problem, but
> > that seems to be abandoned now, and it seems a major undertaking.  While
> > I agree that the proposed solution is a wart, it seems much better than
> > no solution at all.  Can we consider Fujii's proposal as a temporary
> > measure until we fix shared buffers?  I'm +1 on it myself.
> 
> Once we've introduced a user-visible reloption it's going to be
> practically impossible to get rid of it, so I'm -1.  I'd much rather
> see somebody put some effort into the radix-tree idea than introduce
> a kluge that we'll be stuck with, and that doesn't even provide a
> good user experience.  Disabling vacuum truncation is *not* something
> that I think we should recommend.

This new option would not only mitigate the long shared_buffers scan,
it would also get rid of the replication conflict caused by the
AccessExclusiveLock taken during truncation, which is discussed in
https://www.postgresql.org/message-id/c9374921e50a5e8fb1ecf04eb8c6ebc3%40postgrespro.ru
and seems to be a more difficult problem than anticipated.

Could that tip the scales in favor of this stop-gap?

FWIW, I have always considered heap truncation on VACUUM to be something
strange anyway.  VACUUM does not get rid of empty pages in the middle of
a relation, so why is it so important to do it at the end of the relation?
If the answer is "just because we can do it easily", then I think it would be
ok to disable the feature in cases where it causes problems.

Yours,
Laurenz Albe

On 2018-Nov-15, Laurenz Albe wrote:

> This new option would not only mitigate the long shared_buffers scan,
> it would also get rid of the replication conflict caused by the
> AccessExclusiveLock taken during truncation, which is discussed in
> https://www.postgresql.org/message-id/c9374921e50a5e8fb1ecf04eb8c6ebc3%40postgrespro.ru
> and seems to be a more difficult problem than anticipated.

FWIW I was just reminded yesterday that the AEL-for-truncation has been
diagnosed to be a severe problem in production, and with no other
solution in sight, I propose to move forward with the stop-gap.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Nov 15, 2018 at 2:30 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> On 2018-Nov-15, Laurenz Albe wrote:
>
> > This new option would not only mitigate the long shared_buffers scan,
> > it would also get rid of the replication conflict caused by the
> > AccessExclusiveLock taken during truncation, which is discussed in
> > https://www.postgresql.org/message-id/c9374921e50a5e8fb1ecf04eb8c6ebc3%40postgrespro.ru
> > and seems to be a more difficult problem than anticipated.
>
> FWIW I was just reminded yesterday that the AEL-for-truncation has been
> diagnosed to be a severe problem in production, and with no other
> solution in sight, I propose to move forward with the stop-gap.
>

+1.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Thu, Nov 15, 2018 at 03:41:03PM +0530, Amit Kapila wrote:
> On Thu, Nov 15, 2018 at 2:30 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>> FWIW I was just reminded yesterday that the AEL-for-truncation has been
>> diagnosed to be a severe problem in production, and with no other
>> solution in sight, I propose to move forward with the stop-gap.
>>
>
> +1.

+1.
--
Michael

>On Thu, Nov 15, 2018 at 2:30 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>>
>> On 2018-Nov-15, Laurenz Albe wrote:
>>
> > > This new option would not only mitigate the long shared_buffers 
> > > scan, it would also get rid of the replication conflict caused by 
> > > the AccessExclusiveLock taken during truncation, which is discussed 
> > > in 
> > > https://www.postgresql.org/message-id/c9374921e50a5e8fb1ecf04eb8c6eb
> > > c3%40postgrespro.ru and seems to be a more difficult problem than 
> > > anticipated.
> >
> > FWIW I was just reminded yesterday that the AEL-for-truncation has 
> > been diagnosed to be a severe problem in production, and with no other 
> > solution in sight, I propose to move forward with the stop-gap.

I just want to ask whether or not we could proceed with this approach for now and 
if it is possible that we could have this solution integrated before PG12 development ends?

Regards,
Kirk Jamison

From: Jamison, Kirk [mailto:k.jamison@jp.fujitsu.com]
> >On Thu, Nov 15, 2018 at 2:30 PM Alvaro Herrera <alvherre@2ndquadrant.com>
> wrote:
> >>
> >> On 2018-Nov-15, Laurenz Albe wrote:
> >>
> > > > This new option would not only mitigate the long shared_buffers
> > > > scan, it would also get rid of the replication conflict caused by
> > > > the AccessExclusiveLock taken during truncation, which is discussed
> > > > in
> > > >
> https://www.postgresql.org/message-id/c9374921e50a5e8fb1ecf04eb8c6eb
> > > > c3%40postgrespro.ru and seems to be a more difficult problem than
> > > > anticipated.
> > >
> > > FWIW I was just reminded yesterday that the AEL-for-truncation has
> > > been diagnosed to be a severe problem in production, and with no other
> > > solution in sight, I propose to move forward with the stop-gap.
> 
> I just want to ask whether or not we could proceed with this approach for
> now and
> if it is possible that we could have this solution integrated before PG12
> development ends?

As most people seem to agree adding the reloption, here's the patch.  It passes make check, and works like this:

postgres=# CREATE TABLE a (c int) WITH (shrink_enabled  = off);
postgres=# INSERT INTO a VALUES(1);
postgres=# DELETE FROM a;
postgres=# SELECT pg_relation_size('a');
 pg_relation_size
------------------
             8192
(1 row)

postgres=# VACUUM a;
postgres=# SELECT pg_relation_size('a');
 pg_relation_size
------------------
             8192
(1 row)

postgres=#


As Tom said, we want to shorten the shared buffer scan during table truncation as a separate undertaking.  Kirk will do
itfor PG 13.  I'd appreciate much help from many people, because I'm afraid it will be very dificult.
 

And Tom mentioned likewise, I recognize I have to refresh my memory for fixing the data corruption by failed
TRUNCATE...


Regards
Takayuki Tsunakawa

From: Tsunakawa, Takayuki [mailto:tsunakawa.takay@jp.fujitsu.com]
> 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.


Regards
Takayuki Tsunakawa

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.

Thank you for this.
I applied the patch. It builds successfully, and passed the regression tests.
I also tried testing with the parameter when its enabled and disabled,
and it works as intended for CREATE TABLE and ALTER TABLE a SET (shrink_enabled = on/off) and RESET (shrink_enabled).
I have yet to test the performance benchmark.

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 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?

Regards,
Kirk Jamison

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
fromthe 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

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

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

From: Michael Paquier [mailto:michael@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
aboutVACUUM 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'tobject to whatever name any committer chooses.
 


Regards
Takayuki Tsunakawa

Hi,

On Mon, Feb 4, 2019 at 1:25 AM Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
>
> FYI, it seems that the user sees "shrink" rather than "truncate" in the documentation as below, although these are
aboutVACUUM 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'tobject to whatever name any committer chooses.
 

FWIW, I prefer shrink over truncate, though I'd rather go with
vacuum_shink_enabled as suggested previously.

The patch still applies cleanly and works as intended.  About:

+ * shrink_enabled can be set at ShareUpdateExclusiveLock because it
+ * is only used during VACUUM, which uses a ShareUpdateExclusiveLock,
+ * so the VACUUM will not be affected by in-flight changes. Changing its
+ * value has no affect until the next VACUUM, so no need for stronger lock.

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?

From: Julien Rouhaud [mailto:rjuju123@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_enabledbecause this property may be used in other shrink situations in the future.  What I imagined was
thatwith the zheap, DELETE or some maintenance operation, not vacuum, may try to shrink the table.  I meant this
propertyto 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
samecomment block.  The meaning is that the setting cannot be changed during VACUUM, so in-flight VACUUM is not
affected.


Regards
Takayuki Tsunakawa

On Fri, Feb 22, 2019 at 02:38:56AM +0000, Tsunakawa, Takayuki wrote:
> From: Julien Rouhaud [mailto:rjuju123@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

On Fri, Feb 22, 2019 at 3:39 AM Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
>
> No, changing the parameter acquires ShareUpdaeExclusive lock.  I just imitated the description for n_distinct in the
samecomment 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.

From: Michael Paquier [mailto:michael@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:rjuju123@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

On Mon, Feb 25, 2019 at 02:38:05AM +0000, Tsunakawa, Takayuki wrote:
> From: Julien Rouhaud [mailto:rjuju123@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

On Mon, Feb 25, 2019 at 3:56 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Feb 25, 2019 at 02:38:05AM +0000, Tsunakawa, Takayuki wrote:
> > From: Julien Rouhaud [mailto:rjuju123@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

On Mon, Feb 25, 2019 at 7:56 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Feb 25, 2019 at 02:38:05AM +0000, Tsunakawa, Takayuki wrote:
> > From: Julien Rouhaud [mailto:rjuju123@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

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

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

From: Michael Paquier [mailto:michael@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

On Mon, Feb 25, 2019 at 7:01 PM Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
>
> From: Michael Paquier [mailto:michael@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

From: Masahiko Sawada [mailto:sawada.mshk@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

On Tue, Feb 26, 2019 at 3:29 PM Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
>
> From: Masahiko Sawada [mailto:sawada.mshk@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

On Mon, Feb 25, 2019 at 4:25 AM Michael Paquier <michael@paquier.xyz> 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

From: Michael Paquier [mailto:michael@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
parametersalso 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
specifythis, 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
inclinedto separate it.
 


From: Masahiko Sawada [mailto:sawada.mshk@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

From: Robert Haas [mailto:robertmhaas@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
specificdatabases and applications.  But should the vacuuming of system catalogs also follow this setting?
 


Regards
Takayuki Tsunakawa

On Thu, Feb 28, 2019 at 01:05:07AM +0000, Tsunakawa, Takayuki wrote:
> From: Robert Haas [mailto:robertmhaas@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

From: Michael Paquier [mailto:michael@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_shrinkor something like that, but enable_xxx seems to be used solely for planner control.  Plus,
vacuum-relatedparameters 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
takeseffect.
 

* 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
againto 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

On 2019-Feb-28, Tsunakawa, Takayuki wrote:

> From: Michael Paquier [mailto:michael@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_shrinkor something like that, but enable_xxx seems to be used solely for planner control.  Plus,
vacuum-relatedparameters 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 Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> 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.1544474335@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

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

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

From: Alvaro Herrera [mailto:alvherre@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
determinedbased on the application workload, not per VACUUM execution.  Rather, I think GUC is more useful to determine
thebehavior 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

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

On 2019-Feb-28, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> 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.1544474335@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.1544562482@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 Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> 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.1544562482@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

On Thu, Feb 28, 2019 at 3:17 AM Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> 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

On 3/1/19 1:43 PM, Robert Haas wrote:
> On Thu, Feb 28, 2019 at 3:17 AM Tsunakawa, Takayuki
> <tsunakawa.takay@jp.fujitsu.com> 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 Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

Robert Haas <robertmhaas@gmail.com> 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

Andres Freund <andres@anarazel.de> 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

Hi,

On 2019-03-01 14:17:33 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> 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

On 3/1/19 2:14 PM, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> 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 Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Andres Freund <andres@anarazel.de> 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.1551392263@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

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> 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

On Sat, Mar 2, 2019 at 4:34 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> 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

On 3/3/19, 9:23 PM, "Masahiko Sawada" <sawada.mshk@gmail.com> 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

Hi,

On 2019-03-04 20:03:37 +0000, Bossart, Nathan wrote:
> On 3/3/19, 9:23 PM, "Masahiko Sawada" <sawada.mshk@gmail.com> 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

On 3/4/19, 12:11 PM, "Andres Freund" <andres@anarazel.de> 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

Hi,

On 2019-03-04 21:40:53 +0000, Bossart, Nathan wrote:
> On 3/4/19, 12:11 PM, "Andres Freund" <andres@anarazel.de> 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

On 3/4/19, 1:44 PM, "Andres Freund" <andres@anarazel.de> wrote:
> On 2019-03-04 21:40:53 +0000, Bossart, Nathan wrote:
>> On 3/4/19, 12:11 PM, "Andres Freund" <andres@anarazel.de> 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

Hi,

On 2019-03-04 22:00:47 +0000, Bossart, Nathan wrote:
> On 3/4/19, 1:44 PM, "Andres Freund" <andres@anarazel.de> 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

On 3/4/19, 2:05 PM, "Andres Freund" <andres@anarazel.de> wrote:
> On 2019-03-04 22:00:47 +0000, Bossart, Nathan wrote:
>> On 3/4/19, 1:44 PM, "Andres Freund" <andres@anarazel.de> 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

On Tue, Mar 5, 2019 at 5:11 AM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2019-03-04 20:03:37 +0000, Bossart, Nathan wrote:
> > On 3/3/19, 9:23 PM, "Masahiko Sawada" <sawada.mshk@gmail.com> 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

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
david@pgmasters.net

From: David Steele [mailto:david@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
thelatest patch, and replied to the opinions/proposals, so the patch status is kept "needs review."  (I hope new fire
won'thappen...)
 


Regards
Takayuki Tsunakawa

On Tue, Mar 26, 2019 at 3:57 AM Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
> From: David Steele [mailto:david@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
thelatest patch, and replied to the opinions/proposals, so the patch status is kept "needs review."  (I hope new fire
won'thappen...)
 

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

On Tue, Mar 26, 2019 at 10:35 PM Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
> I almost have the same view as Sawada-san.  The reloption vacuum_shrink_enabled is a positive name and follows the
namingstyle 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

From: Robert Haas [mailto:robertmhaas@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
PostgreSQLdocumentation.  So, I chose shrink.  To repeat myself, I'm comfortable with either word.  I'd like the
committerto choose what he thinks better.
 


Regards
Takayuki Tsunakawa

On Thu, Mar 28, 2019 at 11:45 AM Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
>
> From: Robert Haas [mailto:robertmhaas@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
PostgreSQLdocumentation.  So, I chose shrink.  To repeat myself, I'm comfortable with either word.  I'd like the
committerto choose what he thinks better. 

+      This parameter cannot be set for TOAST tables.

reloption for TOAST is also required?

Regards,

--
Fujii Masao

From: Fujii Masao [mailto:masao.fujii@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

On Thu, Apr 4, 2019 at 1:26 PM Tsunakawa, Takayuki
<tsunakawa.takay@jp.fujitsu.com> wrote:
>
> From: Fujii Masao [mailto:masao.fujii@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!

+    <term><literal>vacuum_shrink_enabled</literal>,
<literal>toast.vacuum_shrink_enabled</literal>
(<type>boolean</type>)</term>
+    <listitem>
+     <para>
+     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 <command> or "vacuum" is more appropriate here?

+     Shrinking the table requires <literal>ACCESS EXCLUSIVE</literal>
lock on the table.
+     It can take non-negligible time when the shared buffer is large.
Besides, <literal>ACCESS EXCLUSIVE</literal>
+     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.
+     </para>
+    </listitem>
+   </varlistentry>

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

On Thu, Apr 4, 2019 at 1:23 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Thu, Apr 4, 2019 at 1:26 PM Tsunakawa, Takayuki
> <tsunakawa.takay@jp.fujitsu.com> wrote:
> >
> > From: Fujii Masao [mailto:masao.fujii@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!

> +    <term><literal>vacuum_shrink_enabled</literal>,
> <literal>toast.vacuum_shrink_enabled</literal>
> (<type>boolean</type>)</term>
> +    <listitem>
> +     <para>
> +     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 <command> 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)

From: Masahiko Sawada [mailto:sawada.mshk@gmail.com]
> "VACUUM" needs <command> 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
thespecific 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
settingsfor editing .sgml in Emacs that, for example, puts appropriate number of spaces at the beginning of each line
when<Tab> is pressed, automatically break the long line and put spaces, etc.
 


From: Julien Rouhaud [mailto:rjuju123@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
perVACUUM 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
featureto be included in PG 12, because the vacuum truncaton has been bothering us like others said in this and other
threads...


Regards
Takayuki Tsunakawa

On Thu, Apr 4, 2019 at 10:07 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Thu, Apr 4, 2019 at 1:23 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Thu, Apr 4, 2019 at 1:26 PM Tsunakawa, Takayuki
> > <tsunakawa.takay@jp.fujitsu.com> wrote:
> > >
> > > From: Fujii Masao [mailto:masao.fujii@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!
>
> > +    <term><literal>vacuum_shrink_enabled</literal>,
> > <literal>toast.vacuum_shrink_enabled</literal>
> > (<type>boolean</type>)</term>
> > +    <listitem>
> > +     <para>
> > +     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 <command> 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:
   <para>
    The <option>FULL</option> 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. <command>VACUUM FULL</command> will usually shrink the table
    more than a plain <command>VACUUM</command> would.
   </para>

maintenance.sgml
    Although <command>VACUUM FULL</command> 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
<command>VACUUM</command> runs are a
    better approach than infrequent <command>VACUUM FULL</command> 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

On Thu, Apr 4, 2019 at 9:19 PM Masahiko Sawada <sawada.mshk@gmail.com> 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

On Fri, Apr 5, 2019 at 7:04 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Thu, Apr 4, 2019 at 9:19 PM Masahiko Sawada <sawada.mshk@gmail.com> 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.

On Fri, Apr 5, 2019 at 2:11 PM Julien Rouhaud <rjuju123@gmail.com> 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

Le 5 avril 2019 20:11:38 GMT+02:00, Julien Rouhaud <rjuju123@gmail.com> a écrit :
>On Fri, Apr 5, 2019 at 7:04 PM Robert Haas <robertmhaas@gmail.com>
>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.

On Fri, Apr 5, 2019 at 3:28 PM Adrien Mobile <adrien.nayrat@anayrat.info> 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

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
infrastructurehandle 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

On Fri, Apr 5, 2019 at 3:28 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Fri, Apr 5, 2019 at 2:11 PM Julien Rouhaud <rjuju123@gmail.com> 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 Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Sat, Apr 6, 2019 at 2:04 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Thu, Apr 4, 2019 at 9:19 PM Masahiko Sawada <sawada.mshk@gmail.com> 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

On Mon, Apr 8, 2019 at 9:52 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Sat, Apr 6, 2019 at 2:04 AM Robert Haas <robertmhaas@gmail.com> wrote:
> >
> > On Thu, Apr 4, 2019 at 9:19 PM Masahiko Sawada <sawada.mshk@gmail.com> 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

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

Hi Andres, Fujii-san, any committer,

From: Andres Freund [mailto:andres@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
justin case this is pushed to 13 because of the parameter name...
 


Regards
Takayuki Tsunakawa

"Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> writes:
> From: Andres Freund [mailto:andres@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
justin 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

From: Tom Lane [mailto:tgl@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
witheither name.)  And there's no dispute on the behavior.  Do you see any other point?
 


Regards
Takayuki Tsunakawa

"Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> writes:
> From: Tom Lane [mailto:tgl@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
witheither 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

On Mon, Apr 08, 2019 at 03:56:52AM +0000, 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

Hi,

On 2019-04-08 00:38:44 -0400, Tom Lane wrote:
> "Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com> writes:
> > From: Tom Lane [mailto:tgl@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

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 Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

From: Alvaro Herrera [mailto:alvherre@2ndquadrant.com]
> "vacuum_truncate" gets my vote too.

+1


From: 'Andres Freund' [mailto:andres@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:andres@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

On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> 2019年4月8日(月) 14:22 Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>:
>>
>> From: Alvaro Herrera [mailto:alvherre@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.

On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>:
> >>
> >> From: Alvaro Herrera [mailto:alvherre@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

On Mon, Apr 8, 2019 at 10:15 AM Fujii Masao <masao.fujii@gmail.com> 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

On Mon, Apr 8, 2019 at 5:20 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 10:15 AM Fujii Masao <masao.fujii@gmail.com> 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

On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> >
> > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>:
> > >>
> > >> From: Alvaro Herrera [mailto:alvherre@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

On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > >
> > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > >
> > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>:
> > > >>
> > > >> From: Alvaro Herrera [mailto:alvherre@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

On Mon, Apr 8, 2019 at 7:22 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > > >
> > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > > >
> > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>:
> > > > >>
> > > > >> From: Alvaro Herrera [mailto:alvherre@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

On Mon, Apr 8, 2019 at 12:22 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > > >
> > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > > >
> > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>:
> > > > >>
> > > > >> From: Alvaro Herrera [mailto:alvherre@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 <literal>TRUNCATE</literal> parameter
+      to <xref linkend="sql-vacuum"/>, if specified, overrides the value
+      of this option.

maybe "parameter of" instead of "parameter to"?

On Mon, Apr 8, 2019 at 10:24 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 5:20 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> >
> > On Mon, Apr 8, 2019 at 10:15 AM Fujii Masao <masao.fujii@gmail.com> 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-0000pO-ED%40gemulon.postgresql.org!

From: Fujii Masao [mailto:masao.fujii@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
thedifficulty and fatigue in developing in the community...
 


Regards
Takayuki Tsunakawa

Hello.

At Mon, 8 Apr 2019 19:22:04 +0900, Fujii Masao <masao.fujii@gmail.com> wrote in
<CAHGQGwHa_dX=dRcbR5QVTs6W5mgCy3qZ2fEwREaiXpES1B2+jw@mail.gmail.com>
> > "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

On Tue, Apr 9, 2019 at 1:07 PM Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
>
> Hello.
>
> At Mon, 8 Apr 2019 19:22:04 +0900, Fujii Masao <masao.fujii@gmail.com> wrote in
<CAHGQGwHa_dX=dRcbR5QVTs6W5mgCy3qZ2fEwREaiXpES1B2+jw@mail.gmail.com>
> > > "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

At Wed, 10 Apr 2019 02:00:03 +0900, Fujii Masao <masao.fujii@gmail.com> wrote in
<CAHGQGwGufa=f4RWAscOyz1J76Gs-u+wmUt6oZF8YswLiMF13Ew@mail.gmail.com>
> On Tue, Apr 9, 2019 at 1:07 PM Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> >
> > Hello.
> >
> > At Mon, 8 Apr 2019 19:22:04 +0900, Fujii Masao <masao.fujii@gmail.com> wrote in
<CAHGQGwHa_dX=dRcbR5QVTs6W5mgCy3qZ2fEwREaiXpES1B2+jw@mail.gmail.com>
> > > > "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

Hi,

On 2019-04-08 19:22:04 +0900, Fujii Masao wrote:
> On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada <sawada.mshk@gmail.com> 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

On Mon, Apr 8, 2019 at 7:29 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 7:22 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > >
> > > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > >
> > > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > > > >
> > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > > > >
> > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>:
> > > > > >>
> > > > > >> From: Alvaro Herrera [mailto:alvherre@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

On Tue, May 7, 2019 at 5:33 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 7:29 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Mon, Apr 8, 2019 at 7:22 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > > >
> > > > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > > > > >
> > > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > > > > >
> > > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>:
> > > > > > >>
> > > > > > >> From: Alvaro Herrera [mailto:alvherre@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

On Mon, Apr 8, 2019 at 8:15 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Mon, Apr 8, 2019 at 12:22 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > >
> > > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > >
> > > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
> > > > >
> > > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > > > >
> > > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki <tsunakawa.takay@jp.fujitsu.com>:
> > > > > >>
> > > > > >> From: Alvaro Herrera [mailto:alvherre@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 <literal>TRUNCATE</literal> parameter
> +      to <xref linkend="sql-vacuum"/>, 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

On Wed, May  8, 2019 at 02:14:04AM +0900, Fujii Masao wrote:
> On Tue, May 7, 2019 at 5:33 PM Masahiko Sawada <sawada.mshk@gmail.com> 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  <bruce@momjian.us>        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 +