Since time is short and it seems better to get the xlog page magic bump done before beta2, I've gone ahead and committed this, but there are two points here that seem to warrant some input from other senior hackers. Andres and I have discussed these points off list but without reaching a meeting of the minds.
On Fri, Jun 17, 2016 at 2:59 PM, Robert Haas <robertmh...@gmail.com> wrote: >> Having an XLogInsert() in contrib makes me more than a bit squeamish. I >> think it'd be fair bit better to have that section of code in >> visibilitymap.c, and then call that from the extension. > > I thought about that too, but it seemed like it was just bloating the > core server for no real reason. It's not like contrib is off in space > someplace. So, Andres thinks it's a bad idea to have an XLogInsert() call in contrib, and I don't think it matters. Anyone else have an opinion? Andres, do you want to explain the nature of your concern further? >>> + /* Don't keep the relation locked any longer than necessary! */ >>> + relation_close(rel, AccessExclusiveLock); >> >> Don't think that's a good idea. We use transactional semantics for a >> good reason, and the function returns afterwards anyway. > > Yeah, but if you want to blow away a bunch of visibility map forks at > one go, you're not going to appreciate this function collecting all of > those locks at the same time. This is also why VACUUM starts a > separate transaction for each table. Andres has a concern here that there might be a problem related to invalidation messages. Currently, the only invalidation message that's getting generated here is a non-transactional smgr invalidation, which is sent before the lock is released and therefore no race exists. However, Andres is concerned that this may at a minimum not be very future-proof and possibly unsafe even today. To me, however, this doesn't seem particularly different than what e.g. lazy_truncate_heap() always does and has done for a long time. More input here is welcome and appreciated - it would be good not to screw this up. -- 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
