From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Jan Wieck <JanWieck(at)Yahoo(dot)com> |
Cc: | PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Use of ActiveSnapshot |
Date: | 2007-05-14 17:29:12 |
Message-ID: | 13645.1179163752@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:
> The comment for the call of pg_plan_queries in util/cache/plancache.c
> line 469 for example is fatally wrong. Not only should the snapshot be
> set by all callers at this point, but if the call actually does replan
> the queries, the existing ActiveSnapshot is replaced with one allocated
> on the current memory context. If this happens to be inside of a nested
> SPI call sequence, the innermost SPI stack frame will free the snapshot
> data without restoring ActiveSnapshot to the one from the caller.
Yeah, I'd been meaning to go back and recheck that point after the code
settled down, but forgot :-(.
It is possible for RevalidateCachedPlan to be called with no snapshot
yet set --- at least the protocol Describe messages can do that. I
don't want Describe to force a snapshot because that would be bad for
cases like LOCK TABLE at the start of a serializable transaction, so
RevalidateCachedPlan had better be able to cope with this case.
Since the "typical" case in which no replan is necessary won't touch
the snapshot, I think we'd better adopt the rule that
RevalidateCachedPlan never causes any caller-visible change in
ActiveSnapshot, else we'll be risking very-hard-to-reproduce bugs.
So my proposal is that RevalidateCachedPlan should set a snapshot for
itself if it needs to replan and ActiveSnapshot is NULL (else it might
as well just use the existing snap); and that it should save and restore
ActiveSnapshot when it does this.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Jim C. Nasby | 2007-05-14 17:57:36 | Re: Concurrent psql patch |
Previous Message | Gregory Stark | 2007-05-14 17:26:42 | Re: Concurrent psql patch |