On Fri, Dec 14, 2012 at 7:19 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2012-12-14 14:01:30 -0500, Robert Haas wrote:
>> On Fri, Dec 14, 2012 at 6:46 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>> > Just moving that tidbit inside the lock seems to be the pragmatic
>> > choice. GetOldestXmin is called
>> >
>> > * once per checkpoint
>> > * one per index build
>> > * once in analyze
>> > * twice per vacuum
>> > * once for HS feedback messages
>> >
>> > Nothing of that occurs frequently enough that 5 instructions will make a
>> > difference. I would be happy to go an alternative path, but right now I
>> > don't see any nice one. A "already_locked" parameter to GetOldestXmin
>> > seems to be a cure worse than the disease.
>>
>> I'm not sure that would be so bad, but I guess I question the need to
>> do it this way at all. Most of the time, if you need to advertise
>> your global xmin, you use GetSnapshotData(), not GetOldestXmin(), and
>> I guess I'm not seeing why that wouldn't also work here. Am I dumb?
>
> I wondered upthread whether that would be better:
>
> On 2012-12-13 21:03:44 +0100, Andres Freund wrote:
>> Another alternative to this would be to get a snapshot with
>> GetSnapshotData(), copy the xmin to the logical slot, then call
>> ProcArrayEndTransaction(). But that doesn't really seem to be nicer to
>> me.
>
> Not sure why I considered it ugly anymore, but it actually has a
> noticeable disadvantage. GetOldestXmin is nicer is than GetSnapshotData
> as the latter set a fairly new xid as xmin whereas GetOldestXmin returns
> the actual current xmin horizon. Thats preferrable because it allows us
> to start up more quickly. snapbuild.c can only start building a snapshot
> once it has seen a xl_running_xact with oldestRunningXid >=
> own_xmin. Otherwise we cannot be sure that no relevant catalog tuples
> have been removed.
I'm a bit confused. Are you talking about the difference between
RecentGlobalXmin and RecentXmin? I think GetSnapshotData() updates
both.
Anyway, if there's no nicer way, I think it's probably OK to add a
parameter to GetOldestXmin(). It seems like kind of a hack, though.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company