Обсуждение: Document that vacuum can't truncate if old_snapshot_threshold >= 0

Hi,

Currently, if old_snapshot_threshold is enabled, vacuum is prevented
from truncating tables:
static bool
should_attempt_truncation(LVRelStats *vacrelstats)
{BlockNumber possibly_freeable;
possibly_freeable = vacrelstats->rel_pages - vacrelstats->nonempty_pages;if (possibly_freeable > 0 &&
(possibly_freeable>= REL_TRUNCATE_MINIMUM ||  possibly_freeable >= vacrelstats->rel_pages / REL_TRUNCATE_FRACTION) &&
old_snapshot_threshold < 0)    return true;else    return false;
 
}

(note the old_snapshot_threshold < 0 condition).

That appears to not be mentioned in a comment, the commit message or the
the docs. I think this definitely needs to be prominently documented.

FWIW, afaics that's required because new pages don't have an LSN, so we
can't necessarily detect that a truncated and re-extended relation,
wouldn't be valid. Although I do wonder if there isn't a less invasive
way to do that.

Greetings,

Andres Freund

On Wed, Jul 13, 2016 at 02:14:06PM -0700, Andres Freund wrote:
> That appears to not be mentioned in a comment, the commit message or the
> the docs. I think this definitely needs to be prominently documented.

[Action required within 72 hours.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item.  Kevin,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
efforts toward speedy resolution.  Thanks.

[1] http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com

On Fri, Jul 15, 2016 at 02:38:54AM -0400, Noah Misch wrote:
> On Wed, Jul 13, 2016 at 02:14:06PM -0700, Andres Freund wrote:
> > That appears to not be mentioned in a comment, the commit message or the
> > the docs. I think this definitely needs to be prominently documented.
> 
> [Action required within 72 hours.  This is a generic notification.]
> 
> The above-described topic is currently a PostgreSQL 9.6 open item.  Kevin,
> since you committed the patch believed to have created it, you own this open
> item.  If some other commit is more relevant or if this does not belong as a
> 9.6 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within 72 hours of this
> message.  Include a date for your subsequent status update.  Testers may
> discover new open items at any time, and I want to plan to get them all fixed
> well in advance of shipping 9.6rc1.  Consequently, I will appreciate your
> efforts toward speedy resolution.  Thanks.
> 
> [1] http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com

This PostgreSQL 9.6 open item is past due for your status update.  Kindly send
a status update within 24 hours, and include a date for your subsequent status
update.  Refer to the policy on open item ownership:
http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com

On Wed, Jul 13, 2016 at 4:14 PM, Andres Freund <andres@anarazel.de> wrote:

> Currently, if old_snapshot_threshold is enabled, vacuum is prevented
> from truncating tables:
> static bool
> should_attempt_truncation(LVRelStats *vacrelstats)
> {
>         BlockNumber possibly_freeable;
>
>         possibly_freeable = vacrelstats->rel_pages - vacrelstats->nonempty_pages;
>         if (possibly_freeable > 0 &&
>                 (possibly_freeable >= REL_TRUNCATE_MINIMUM ||
>           possibly_freeable >= vacrelstats->rel_pages / REL_TRUNCATE_FRACTION) &&
>                 old_snapshot_threshold < 0)
>                 return true;
>         else
>                 return false;
> }
>
> (note the old_snapshot_threshold < 0 condition).
>
> That appears to not be mentioned in a comment, the commit message or the
> the docs. I think this definitely needs to be prominently documented.
>
> FWIW, afaics that's required because new pages don't have an LSN, so we
> can't necessarily detect that a truncated and re-extended relation,
> wouldn't be valid. Although I do wonder if there isn't a less invasive
> way to do that.

There would be no problem on re-extended pages because they would
have a new enough LSN to cause a "snapshot too old" error; it is
when they are missing that the problem exists.  The possible
alternative that looked best to me was to leave a "guard page" to
cover sequential scans, with LSN set to (at least) the latest of
itself or any truncated page.  I think WAL would need to be
generated to cover the LSN update.  If you combined that with
treating an index leaf tuple pointing to a missing page as
"snapshot too old" I think things would work, but it's not clear to
me that it's worth the complexity.

Attached pushed.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company