On Wed, Nov 8, 2017 at 2:26 AM, Jacob Champion <pchampion@pivotal.io> wrote:
> On Mon, Nov 6, 2017 at 6:18 PM, Michael Paquier
> <michael.paquier@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
