On Thu, Jun 9, 2016 at 10:28 AM, Kevin Grittner <kgri...@gmail.com> wrote: > [Thanks to Robert to stepping up to keep this moving while I was > down yesterday with a minor injury. I'm back on it today.] >> Generally, I think I see the hazard you're concerned about: I had >> failed to realize, as you mentioned upthread, that new index pages >> would have an LSN of 0. So if a tuple is pruned early and then the >> index is reindexed, old snapshots won't realize that data is missing. >> What I'm less certain about is whether you can actually get by with >> reusing ii_BrokenHotChain to handle this case. > > v2 and later does not do that. v1 did, but that was a more blunt > instrument. > >> For example, note this comment: >> >> * However, when reindexing an existing index, we should do nothing here. >> * Any HOT chains that are broken with respect to the index must predate >> * the index's original creation, so there is no need to change the >> * index's usability horizon. Moreover, we *must not* try to change the >> * index's pg_index entry while reindexing pg_index itself, and this >> * optimization nicely prevents that. >> >> This logic doesn't apply to the old snapshot case; there, you'd need >> to distrust the index whether it was an initial build or a REINDEX, >> but it doesn't look like that's what the patch does. I'm worried >> there could be other places where we rely on ii_BrokenHotChain to >> detect only one specific condition that isn't quite the same as what >> you're trying to use it for here. > > Well spotted. I had used a lot of discreet calls to get that > reindex logic right, but it was verbose and ugly, so I had just > added the new macros in this patch and started to implement them > before I knocked off for the day. At handover I was too distracted > to remember where I was with it. :-( See if it looks right to you > now. > > Attached is v3. I will commit this patch to resolve this issue > tomorrow, barring any objections before then.
So I like the idea of centralizing checks in RelationAllowsEarlyVacuum, but shouldn't it really be called RelationAllowsEarlyPruning? Will look at this a bit more if I get time. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
