[Reposting with compressed patch]
Okay, I think I've fixed most of the issues in the reviewed patch.
Updated patch attached.
The most interesting change is that I've done away with CopySnapshot as
a public routine in favor of a new PushUpdatedSnapshot which does the
copy-update-push sequence. Also I added a refcount to RegdSnapshotElt
as suggested, and changed the subxact logic to substract that exact
amount on abort.
There's something I'm not sure what to do about:
Tom Lane wrote:
> Also, I think that the whole snapshot-sharing mechanism is not working
> as intended except for the serializable case; otherwise sequences
> like
> x = RegisterSnapshot(GetTransactionSnapshot());
> y = RegisterSnapshot(GetTransactionSnapshot());
> will result in x and y being separate copies. Or are you assuming
> that this just isn't worth optimizing?
It's not that I don't think it's worth optimizing, but I think it's a
bit away from the scope of this patch. The problem here is how to
notice that two consecutive GetTransactionSnapshot calls should really
return different snapshots, considering that shared state may change in
between. Perhaps there's an easy way to optimize that; I don't know.
What does work is to get (say) a registered snapshot and push it as
active snapshot. That results in a successfully shared snapshot. For
example PortalStart does that for cursors, etc.
(FWIW another thing which is probably worth rethinking is the handling
of snapshots around PortalStart. Some callers pass the currently active
snapshot; Others pass InvalidSnapshot. Another passes an arbitrary
snapshot. When it's Invalid, PortalStart calls GetTransactionSnapshot,
otherwise it uses the passed snap for PushActiveSnapshot. So this is
all a bit confusing and wasteful and could use some clean up. This is
material for a new patch however.)
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support