Tom Lane wrote:
Patch committed, with most of these fixed. On this item:
> AtSubCommit_Snapshot can leave us with multiple RegdSnapshotElt's for
> the same snap and s_level, which seems a bit bogus.
I agree it is bogus. I punted though, and left the code as-is, with a
comment stating the probl
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Hmm ... but that "close" can't unregister the snapshot immediately,
>> because you'd lose if the 2nd savepoint gets rolled back, no? Is the
>> handling of this case even correct at the moment?
> No, CLOSE is not rolled back:
> ...
>
Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > Tom Lane wrote:
> >> Shouldn't UnregisterSnapshot insist that s_level be equal to current
> >> xact nest level?
>
> > It can't check that; consider
>
> > begin;
> > savepoint foo;
> > declare cur cursor for select (1), (2), (3);
> >
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Shouldn't UnregisterSnapshot insist that s_level be equal to current
>> xact nest level?
> It can't check that; consider
> begin;
> savepoint foo;
> declare cur cursor for select (1), (2), (3);
> savepoint bar;
> close cur;
> commit;
Tom Lane wrote:
I'm revising the patch; this comment is flawed though:
> Shouldn't UnregisterSnapshot insist that s_level be equal to current
> xact nest level?
It can't check that; consider
begin;
savepoint foo;
declare cur cursor for select (1), (2), (3);
savepoint bar;
close cur;
commit;
Th
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> 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(GetTransacti
[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. Al
Tom Lane wrote:
> I looked over this patch and I think it still needs work.
Thanks for the thorough review. I'll be working on these problems soon.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 suppor
I wrote:
> Also, I don't think the subtransaction management is correct at all.
BTW, maybe it would make more sense to keep the reference count
management inside ResourceOwners, instead of having a largely
duplicative set of logic in snapmgr.c.
regards, tom lane
--
Sent
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Simon Riggs wrote:
>> On Tue, 2008-04-22 at 15:49 -0400, Alvaro Herrera wrote:
>>> - Three CopySnapshot call sites remain outside snapmgr.c: DoCopy() on
>>> copy.c, ExplainOnePlan() on explain.c and _SPI_execute_plan() on spi.c.
>>> They are there becaus
Alvaro Herrera wrote:
> FWIW I noticed yesterday after going to bed that SnapshotContext serves
> no useful purpose -- we can just remove it and store snaps in
> TopTransactionContext.
... which is what the attached patch does.
--
Alvaro Herrerahttp://www.Command
Simon Riggs wrote:
> Forgive me for being dense, but what is there to stop you using a
> CopySnapshot in TopMemoryContext? If you did, there would be no way to
> free it, nor would we notice it had been done, AFAICS. Not anything I'm
> thinking about doing, though.
Well, TopMemoryContext is no go
On Wed, 2008-04-23 at 08:21 -0400, Alvaro Herrera wrote:
> Simon Riggs wrote:
> > On Tue, 2008-04-22 at 18:13 -0400, Alvaro Herrera wrote:
> > > Simon Riggs wrote:
> > >
> > > > OK, so it can;t be copied to a longer lived memory context?
> > >
> > > If you need that ability, please explain.
> >
Tom Lane wrote:
> The only reason we have memory contexts at all is to avoid the need to
> track individual palloc'd objects. Since we're instituting exactly such
> tracking for snapshots, there's no value in placing them in
> general-purpose memory contexts.
FWIW I noticed yesterday after going
Simon Riggs wrote:
> On Tue, 2008-04-22 at 18:13 -0400, Alvaro Herrera wrote:
> > Simon Riggs wrote:
> >
> > > OK, so it can;t be copied to a longer lived memory context?
> >
> > If you need that ability, please explain.
>
> No, I wish to prevent that, not enable it.
I see. Sure, we don't have
On Tue, 2008-04-22 at 18:13 -0400, Alvaro Herrera wrote:
> Simon Riggs wrote:
>
> > OK, so it can;t be copied to a longer lived memory context?
>
> CopySnapshot always copies snapshots to SnapshotContext, which is a
> context that lives until transaction end. There's no mechanism for
> copying a
Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
>
> > CopySnapshot always copies snapshots to SnapshotContext, which is a
> > context that lives until transaction end. There's no mechanism for
> > copying a snapshot into another context, because I don't see the need.
>
> The only re
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Simon Riggs wrote:
>> OK, so it can;t be copied to a longer lived memory context?
> CopySnapshot always copies snapshots to SnapshotContext, which is a
> context that lives until transaction end. There's no mechanism for
> copying a snapshot into anoth
Simon Riggs wrote:
> OK, so it can;t be copied to a longer lived memory context?
CopySnapshot always copies snapshots to SnapshotContext, which is a
context that lives until transaction end. There's no mechanism for
copying a snapshot into another context, because I don't see the need.
If you n
On Tue, 2008-04-22 at 17:50 -0400, Alvaro Herrera wrote:
> Simon Riggs wrote:
> > On Tue, 2008-04-22 at 15:49 -0400, Alvaro Herrera wrote:
> >
> > > - Three CopySnapshot call sites remain outside snapmgr.c: DoCopy() on
> > > copy.c, ExplainOnePlan() on explain.c and _SPI_execute_plan() on spi.c.
>
Simon Riggs wrote:
> On Tue, 2008-04-22 at 15:49 -0400, Alvaro Herrera wrote:
>
> > - Three CopySnapshot call sites remain outside snapmgr.c: DoCopy() on
> > copy.c, ExplainOnePlan() on explain.c and _SPI_execute_plan() on spi.c.
> > They are there because they grab the current ActiveSnapshot, mod
On Tue, 2008-04-22 at 15:49 -0400, Alvaro Herrera wrote:
> - Three CopySnapshot call sites remain outside snapmgr.c: DoCopy() on
> copy.c, ExplainOnePlan() on explain.c and _SPI_execute_plan() on spi.c.
> They are there because they grab the current ActiveSnapshot, modify it,
> and then use the re
Here is the patch for snapshot management I currently have.
Below is a complete description of the changes in this patch.
The most important change is on the use of CopySnapshot and the games
with ActiveSnapshot. On the new code, whenever a piece of code needs a
snapshot to persist, it is "regis
23 matches
Mail list logo
