getting rid of SnapshotNow

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Cc: pgsql-odbc(at)postgresql(dot)org
Subject: getting rid of SnapshotNow
Date: 2013-07-18 12:46:48
Message-ID: CA+TgmoaLeu7wKtfPA0v=iQNMKOJZqg4TZf1Gyxn=7mbwa=Mxag@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-odbc

There seems to be a consensus that we should try to get rid of
SnapshotNow entirely now that we have MVCC catalog scans, so I'm
attaching two patches that together come close to achieving that goal:

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.

2. snapshot-self-not-now-v1.patch changes several uses of SnapshotNow
to use SnapshotSelf instead. These include pgrowlocks(),
pgstat_heap(), and get_actual_variable_range(). In all of those
cases, only an approximately-correct answer is needed, so the change
should be fine. I'd also generally expect that it's very unlikely for
any of these things to get called in a context where the table being
scanned has been updated by the current transaction after the most
recent command-counter increment, so in practice the change in
semantics will probably not be noticeable at all.

Barring objections, I'll commit both of these next week.

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. If I were a betting man, I'd
bet that they are used in contexts where the difference between
SnapshotNow and SnapshotSelf wouldn't matter there, either. For
example, if those functions are always invoked in a query that does
nothing but call those functions, the difference wouldn't be visible.
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.

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

Attachment Content-Type Size
snapshot-error-v1.patch application/octet-stream 3.4 KB
snapshot-self-not-now-v1.patch application/octet-stream 2.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Merlin Moncure 2013-07-18 12:57:33 Re: Return of "can't paste into psql" issue
Previous Message Fabien COELHO 2013-07-18 11:46:11 Re: Add some regression tests for SEQUENCE

Browse pgsql-odbc by date

  From Date Subject
Next Message Tom Lane 2013-07-18 14:34:14 Re: [ODBC] getting rid of SnapshotNow
Previous Message Dev Kumkar 2013-07-16 11:06:46 Re: Fwd: configure: error: unixODBC library "odbcinst" not found while trying to compile odbc