getting rid of SnapshotNow

Robert Haas <robertmhaas@gmail.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.

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?

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.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Jul 18, 2013 at 10:34 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.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

--
Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-odbc

Robert Haas escribi�:

On Thu, Jul 18, 2013 at 10:34 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.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.

Yeah ... SnapshotError is a way to ensure the server doesn't crash if an
extension hasn't been fixed in order not to cause a crash if it doesn't
use the APIs correctly. However, there's many other ways for a
C-language extension to cause crashes, so I don't think this is buying
us much.

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.

They don't show up in a quick grep of psqlodbc's source code, FWIW.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Jul 18, 2013 at 11:54 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

They don't show up in a quick grep of psqlodbc's source code, FWIW.

Hmm. Maybe we should just remove them and see if anyone complains.
We could always put them back (or make them available via contrib) if
it's functionality someone actually needs. The last discussion of
those functions was in 2007 and nobody seemed too sure back then
either, so maybe the rumor that anyone is actually using this is no
more than rumor.

--
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

On 2013-07-18 12:01:39 -0400, Robert Haas wrote:

On Thu, Jul 18, 2013 at 11:54 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

They don't show up in a quick grep of psqlodbc's source code, FWIW.

Hmm. Maybe we should just remove them and see if anyone complains.
We could always put them back (or make them available via contrib) if
it's functionality someone actually needs. The last discussion of
those functions was in 2007 and nobody seemed too sure back then
either, so maybe the rumor that anyone is actually using this is no
more than rumor.

I am pretty sure they are still used. A quick grep on a not too old
checkout prooves that... Note that the sql accessible functions are
named currtid and currtid2 (yes, really)...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund escribi�:

On 2013-07-18 12:01:39 -0400, Robert Haas wrote:

On Thu, Jul 18, 2013 at 11:54 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

They don't show up in a quick grep of psqlodbc's source code, FWIW.

Hmm. Maybe we should just remove them and see if anyone complains.
We could always put them back (or make them available via contrib) if
it's functionality someone actually needs. The last discussion of
those functions was in 2007 and nobody seemed too sure back then
either, so maybe the rumor that anyone is actually using this is no
more than rumor.

I am pretty sure they are still used. A quick grep on a not too old
checkout prooves that... Note that the sql accessible functions are
named currtid and currtid2 (yes, really)...

Ah, yeah, that does show up. I had grepped for 'currtid_'. Sorry.
They're all in positioned_load() in results.c.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-odbc

On Thu, Jul 18, 2013 at 12:25 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Ah, yeah, that does show up. I had grepped for 'currtid_'. Sorry.
They're all in positioned_load() in results.c.

Well, in that case, we'll have to keep it around. I still wish we
could get a clear answer to the question of how it's being used.

--
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

(2013/07/18 23:54), Robert Haas wrote:

On Thu, Jul 18, 2013 at 10:34 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.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.

Though PostgreSQL's TID is similar to Orale's ROWID, it is transient
and changed after update operations unfortunately. I implemented
the currtid_xx functions to supplement the difference. For example

currtid(relname, original tid)

(hopefully) returns the current tid of the original row when it is
updated.

regards,
Hiroshi Inoue

--
Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-odbc

On Thu, Jul 18, 2013 at 08:46:48AM -0400, Robert Haas wrote:

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.

I don't have a strong opinion. Anything it diagnoses is a code bug, probably
one that makes the affected extension useless until it's fixed. But the patch
is small and self-contained. I think the benefit, more than making things
safer in production, would be reducing the amount of time the developer needs
to zero in on the problem. It wouldn't be the first time we've done that;
compare AtEOXact_Buffers(). Does this particular class of bug deserve that
aid? I don't know.

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.

SnapshotSelf is awfully special; currently, you can grep for all uses of it
and find a collection of callers with highly-technical needs. Diluting that
with a handful of callers that legitimately preferred SnapshotNow but don't
care enough to mind SnapshotSelf in its place brings a minor loss of clarity.

From an accuracy perspective, GetActiveSnapshot() does seem ideal for
get_actual_variable_range(). That's independent of any hurry to remove
SnapshotNow. A possible disadvantage is that older snapshots could waste time
scanning back through newer index entries, when a more-accessible value would
be good enough for estimation purposes.

To me, the major advantage of removing SnapshotNow is to force all third-party
code to reevaluate. But that could be just as well achieved by renaming it
to, say, SnapshotImmediate. If there are borderline-legitimate SnapshotNow
uses in our code base, I'd lean toward a rename instead. Even if we decide to
remove every core use, third-party code might legitimately reach a different
conclusion on similar borderline cases.

Thanks,
nm

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com

--
Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-odbc

Noah Misch <noah@leadboat.com> writes:

To me, the major advantage of removing SnapshotNow is to force all
third-party code to reevaluate. But that could be just as well
achieved by renaming it to, say, SnapshotImmediate. If there are
borderline-legitimate SnapshotNow uses in our code base, I'd lean
toward a rename instead. Even if we decide to remove every core use,
third-party code might legitimately reach a different conclusion on
similar borderline cases.

Meh. If there is third-party code with a legitimate need for
SnapshotNow, all we'll have done is to create an annoying version
dependency for them. So if we think that's actually a likely scenario,
we shouldn't rename it. But the entire point of this change IMO is that
we *don't* think there is a legitimate use-case for SnapshotNow.

Indeed, I'm thinking I don't believe in SnapshotSelf anymore either.
It's got all the same consistency issues as SnapshotNow. In fact, it
has *more* issues, because it's also vulnerable to weirdnesses caused by
inconsistent ordering of tuple updates among multiple tuples updated by
the same command.

Why not tell people to use SnapshotDirty if they need a
not-guaranteed-consistent result? At least then it's pretty obvious
that you're getting some randomness in with your news.

regards, tom lane

--
Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-odbc

On Fri, Jul 19, 2013 at 1:27 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Noah Misch <noah@leadboat.com> writes:

To me, the major advantage of removing SnapshotNow is to force all
third-party code to reevaluate. But that could be just as well
achieved by renaming it to, say, SnapshotImmediate. If there are
borderline-legitimate SnapshotNow uses in our code base, I'd lean
toward a rename instead. Even if we decide to remove every core use,
third-party code might legitimately reach a different conclusion on
similar borderline cases.

Meh. If there is third-party code with a legitimate need for
SnapshotNow, all we'll have done is to create an annoying version
dependency for them. So if we think that's actually a likely scenario,
we shouldn't rename it. But the entire point of this change IMO is that
we *don't* think there is a legitimate use-case for SnapshotNow.

Indeed, I'm thinking I don't believe in SnapshotSelf anymore either.
It's got all the same consistency issues as SnapshotNow. In fact, it
has *more* issues, because it's also vulnerable to weirdnesses caused by
inconsistent ordering of tuple updates among multiple tuples updated by
the same command.

Why not tell people to use SnapshotDirty if they need a
not-guaranteed-consistent result? At least then it's pretty obvious
that you're getting some randomness in with your news.

You know, I didn't really consider that before, but I kind of like it.
I think that would be entirely suitable (and perhaps better) for
pgstattuple and get_actual_variable_range().

On further reflection, I think perhaps pgrowlocks should just register
a fresh MVCC snapshot and use that. Using SnapshotDirty would return
TIDs of unseen tuples, which does not seem to be what is wanted there.

--
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

On 2013-07-19 08:57:01 +0900, Inoue, Hiroshi wrote:

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.

Though PostgreSQL's TID is similar to Orale's ROWID, it is transient
and changed after update operations unfortunately. I implemented
the currtid_xx functions to supplement the difference. For example

currtid(relname, original tid)

(hopefully) returns the current tid of the original row when it is
updated.

That is only guaranteed to work though when you're in a transaction old
enough to prevent removal of the old or intermediate row versions. E.g.
BEGIN;
INSERT INTO foo...; -- last tid (0, 1)
COMMIT;
BEGIN;
SELECT currtid(foo, '(0, 1'));
COMMIT;

can basically return no or even an arbitrarily different row. Same with
an update...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-odbc

On 2013-07-19 01:27:41 -0400, Tom Lane wrote:

Noah Misch <noah@leadboat.com> writes:

To me, the major advantage of removing SnapshotNow is to force all
third-party code to reevaluate. But that could be just as well
achieved by renaming it to, say, SnapshotImmediate. If there are
borderline-legitimate SnapshotNow uses in our code base, I'd lean
toward a rename instead. Even if we decide to remove every core use,
third-party code might legitimately reach a different conclusion on
similar borderline cases.

I don't think there are many people that aren't active on -hackers that
can actually understand the implications of using SnapshotNow. Given
-hackers hasn't fully grasped them in several cases... And even if those
borderline cases are safe, that's really only valid for a specific
postgres version. Catering to that doesn't seem like a good idea to me.

Indeed, I'm thinking I don't believe in SnapshotSelf anymore either.
It's got all the same consistency issues as SnapshotNow. In fact, it
has *more* issues, because it's also vulnerable to weirdnesses caused by
inconsistent ordering of tuple updates among multiple tuples updated by
the same command.

Hm. I kind of can see the point of it in constraint code where it
probably would be rather hard to remove usage of it, but e.g. the
sepgsql usage looks pretty dubious to me.
At least in the cases where the constraint code uses them I don't think
the SnapshotNow dangers apply since those specific rows should be locked
et al.

The selinux usage looks like a design flaw to me, but I don't really
understand that code, so I very well may be wrong.

Why not tell people to use SnapshotDirty if they need a
not-guaranteed-consistent result? At least then it's pretty obvious
that you're getting some randomness in with your news.

Especially if we're going to lower the lock level of some commands, but
even now, that opens us to more issues due to nonmatching table
definitions et al. That doesn't sound like a good idea to me.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-odbc

(2013/07/19 22:03), Andres Freund wrote:

On 2013-07-19 08:57:01 +0900, Inoue, Hiroshi wrote:

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.

Though PostgreSQL's TID is similar to Orale's ROWID, it is transient
and changed after update operations unfortunately. I implemented
the currtid_xx functions to supplement the difference. For example

currtid(relname, original tid)

(hopefully) returns the current tid of the original row when it is
updated.

That is only guaranteed to work though when you're in a transaction old
enough to prevent removal of the old or intermediate row versions. E.g.

Yes it's what I meant by (hopefully).
At the time when I implemented currtid(), I was able to use TIDs in
combination with OIDs.

regards,
Hiroshi Inoue

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

(2013/07/18 21:46), Robert Haas wrote:

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:

...

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.

Using SnapshotSelf instead of SnapshotNow for currtid_ () wouldn't
matter.

regards,
Hiroshi Inoue

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2013-07-20 00:49:11 +0900, Hiroshi Inoue wrote:

(2013/07/18 21:46), Robert Haas wrote:

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:

...

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.

Using SnapshotSelf instead of SnapshotNow for currtid_ () wouldn't
matter.

I think it actually might. You could get into dicey situations if you
use currtid_ in a query performing updates or inserts because it would
see the to-be-inserted tuple...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-odbc mailing list (pgsql-odbc@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-odbc

Andres Freund <andres@2ndquadrant.com> writes:

On 2013-07-20 00:49:11 +0900, Hiroshi Inoue wrote:

Using SnapshotSelf instead of SnapshotNow for currtid_ () wouldn't
matter.

I think it actually might. You could get into dicey situations if you
use currtid_ in a query performing updates or inserts because it would
see the to-be-inserted tuple...

I'm pretty sure Hiroshi-san was only opining about whether it would
matter for ODBC's usage. IIUC, ODBC is using this function to re-fetch
rows that it inserted, updated, or at least selected-for-update in a
previous command of the current transaction, so actually any snapshot
would do fine.

In any case, since I moved the goalposts by suggesting that SnapshotSelf
is just as dangerous as SnapshotNow, what we need to know is whether
it'd be all right to change this code to use a fresh MVCC snapshot;
and if not, why not. It's pretty hard to see a reason why client-side
code would want to make use of the results of a non-MVCC snapshot.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas escribi�:

On Fri, Jul 19, 2013 at 1:27 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Why not tell people to use SnapshotDirty if they need a
not-guaranteed-consistent result? At least then it's pretty obvious
that you're getting some randomness in with your news.

On further reflection, I think perhaps pgrowlocks should just register
a fresh MVCC snapshot and use that. Using SnapshotDirty would return
TIDs of unseen tuples, which does not seem to be what is wanted there.

I think seeing otherwise invisible rows is useful in pgrowlocks. It
helps observe the effects on tuples written by concurrent transactions
during experimentation. But then, maybe this functionality belongs in
pageinspect instead.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Jul 19, 2013 at 12:29 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Robert Haas escribió:

On Fri, Jul 19, 2013 at 1:27 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Why not tell people to use SnapshotDirty if they need a
not-guaranteed-consistent result? At least then it's pretty obvious
that you're getting some randomness in with your news.

On further reflection, I think perhaps pgrowlocks should just register
a fresh MVCC snapshot and use that. Using SnapshotDirty would return
TIDs of unseen tuples, which does not seem to be what is wanted there.

I think seeing otherwise invisible rows is useful in pgrowlocks. It
helps observe the effects on tuples written by concurrent transactions
during experimentation. But then, maybe this functionality belongs in
pageinspect instead.

It does seem like it should be useful, at least as an option. But I
feel like changing that ought to be separate from getting rid of
SnapshotNow. It seems like too big of a behavior change to pass off
as a harmless tweak.

--
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