Re: getting rid of SnapshotNow

On Thu, Jul 18, 2013 at 10:34 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> 1. snapshot-error-v1.patch introduces a new special snapshot, called
>> SnapshotError. In the cases where we set SnapshotNow as a sort of
>> default snapshot, this patch changes the code to use SnapshotError
>> instead. This affects scan->xs_snapshot in genam.c and
>> estate->es_snapshot in execUtils.c. This passes make check-world, so
>> apparently there is no code in the core distribution that does this.
>> However, this is safer for third-party code, which will ERROR instead
>> of seg faulting. The alternative approach would be to use
>> InvalidSnapshot, which I think would be OK too if people dislike this
>> approach.
>
> FWIW, I think using InvalidSnapshot would be preferable to introducing
> a new concept for what's pretty much the same thing.

Andres voted the other way on the previous thread. I'll wait and see
if there are any other opinions. The SnapshotError concept seemed
attractive to me initially, but I'm not as excited about it after
seeing how it turned out, so maybe it's best to do it as you suggest.

>> With that done, the only remaining uses of SnapshotNow in our code
>> base will be in currtid_byreloid() and currtid_byrelname(). So far no
>> one on this list has been able to understand clearly what the purpose
>> of those functions is, so I'm copying this email to pgsql-odbc in case
>> someone there can provide more insight.
>
> I had the idea they were used for a client-side implementation of WHERE
> CURRENT OF. Perhaps that's dead code and could be removed entirely?

It's been reported that ODBC still uses them.

>> If we don't want to risk any change to the semantics, we can (1) grit
>> our teeth and keep SnapshotNow around or (2) use an instantaneous MVCC
>> snapshot there, and accept that people who have very large connection
>> counts and extremely heavy use of those functions may see a
>> performance regression.
>
> Of those I'd go for (2); it's unlikely that an extra snapshot would be
> visible compared to the query roundtrip overhead. But another attractive
> possibility is to make these functions use GetActiveSnapshot(), which
> would avoid taking any new snapshot.

You could probably construct a case where the overhead is visible, if
you ran the functions many times in a single query, but arguably no
one does that. Also, Andres's test case that involves running BEGIN;
SELECT txid_current(); very short sleep; COMMIT; in several hundred
sessions at once is pretty brutal on PGXACT and makes the overhead of
taking extra snapshots a lot more visible.

I'm not too familiar with GetActiveSnapshot(), but wouldn't that
change the user-visible semantics? If, for example, someone's using
that function to test whether the row has been updated since their
snapshot was taken, that use case would get broken. SnapshotSelf
would be change from the current behavior in many fewer cases than
using an older snapshot.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company