On Wed, Nov 8, 2017 at 2:26 AM, Jacob Champion <pchamp...@pivotal.io> wrote: > On Mon, Nov 6, 2017 at 6:18 PM, Michael Paquier > <michael.paqu...@gmail.com> wrote: >> Did you really test WAL replay? > > Is there a way to test this other than installcheck-world? The only > failure we've run into at the moment is in the snapshot-too-old tests. > Maybe we're not configuring with all the tests enabled?
Not automatically. The simplest method I use in this case is installcheck with a standby replaying changes behind. >>> The assertion added caught at least one code path where TestForOldSnapshot >>> calls PageGetLSN without holding any lock. The snapshot_too_old test in >>> "check-world" failed due to the assertion failure. This needs to be fixed, >>> see the open question in the opening mail on this thread. >> >> Good point. I am looping Kevin Grittner here for his input, as the >> author and committer of old_snapshot_threshold. Things can be >> addressed with a separate patch by roughly moving the check on >> PageGetLSN() to TestForOldSnapshot_impl() and use BufferGetLSNAtomic() >> instead. > > It still doesn't pass the tests, as not all code paths ensure that a > content lock is held before calling TestForOldSnapshot. > BufferGetLSNAtomic only adds the spinlock. I would prefer waiting for more input from Kevin here. This may prove to be a more invasive change. There are no objections into fixing the existing callers in index AMs though. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
