On Wed, Jul 13, 2016 at 4:14 PM, Andres Freund <and...@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
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 4e8c982..c9e0ec2 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2092,6 +2092,15 @@ include_dir 'conf.d' </para> <para> + When this feature is enabled, freed space at the end of a relation + cannot be released to the operating system, since that could remove + information needed to detect the <literal>snapshot too old</> + condition. All space allocated to a relation remains associated with + that relation for reuse only within that relation unless explicitly + freed (for example, with <command>VACUUM FULL</>). + </para> + + <para> This setting does not attempt to guarantee that an error will be generated under any particular circumstances. In fact, if the correct results can be generated from (for example) a cursor which diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c index 4075f4d..231e92d 100644 --- a/src/backend/commands/vacuumlazy.c +++ b/src/backend/commands/vacuumlazy.c @@ -1663,6 +1663,15 @@ lazy_cleanup_index(Relation indrel, * Don't even think about it unless we have a shot at releasing a goodly * number of pages. Otherwise, the time taken isn't worth it. * + * Also don't attempt it if we are doing early pruning/vacuuming, because a + * scan which cannot find a truncated heap page cannot determine that the + * snapshot is too old to read that page. We might be able to get away with + * truncating all except one of the pages, setting its LSN to (at least) the + * maximum of the truncated range if we also treated an index leaf tuple + * pointing to a missing heap page as something to trigger the "snapshot too + * old" error, but that seems fragile and seems like it deserves its own patch + * if we consider it. + * * This is split out so that we can test whether truncation is going to be * called for before we actually do it. If you change the logic here, be * careful to depend only on fields that lazy_scan_heap updates on-the-fly.
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers