From: | Simon Riggs <simon(at)2ndquadrant(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Bernd Helmle <mailings(at)oopsware(dot)de>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Should we cacheline align PGXACT? |
Date: | 2017-02-20 17:19:05 |
Message-ID: | CANP8+jJdXE9b+b9F8CQT-LuxxO0PBCB-SZFfMVAdp+akqo4zfg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 20 February 2017 at 16:53, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Mon, Feb 20, 2017 at 6:02 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> On 15 February 2017 at 19:15, Andres Freund <andres(at)anarazel(dot)de> wrote:
>>
>>> I think I previously
>>> mentioned, even just removing the MyPgXact->xmin assignment in
>>> SnapshotResetXmin() is measurable performance wise and cache-hit ratio
>>> wise.
>>
>> Currently, we issue SnapshotResetXmin() pointlessly at end of xact, so
>> patch attached to remove that call, plus some comments to explain
>> that. This reduces the cause.
>>
>> Also, another patch to reduce the calls to SnapshotResetXmin() using a
>> simple heuristic to reduce the effects.
>
> I think skip_SnapshotResetXmin_if_idle_timeout.v1.patch isn't a good
> idea, because it could have the surprising result that setting
> idle_in_transaction_timeout to a non-zero value makes bloat worse. I
> don't think users will like that.
>
> Regarding reduce_pgxact_access_AtEOXact.v1.patch, it took me a few
> minutes to figure out that the comment was referring to
> ProcArrayEndTransaction(), so it might be good to be more explicit
> about that if we go forward with this.
Sure, attached.
> Have you checked whether this
> patch makes any noticeable performance difference?
No, but then we're reducing the number of calls to PgXact directly;
there is no heuristic involved, its just a pure saving.
> It's sure
> surprising that we go to all of this trouble to clean things up in
> AtEOXact_Snapshot() when we've already nuked MyPgXact->xmin from
> orbit. (Instead of changing AtEOXact_Snapshot, should we think about
> removing the xid clear logic from ProcArrayEndTransaction and only
> doing it here, or would that be wrong-headed?)
If anything, I'd move the call to PgXact->xmin = InvalidTransactionId
into a function inside procarray.c, so we only touch snapshots in
snapmgr.c and all procarray stuff is isolated. (Not done here, yet).
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment | Content-Type | Size |
---|---|---|
reduce_pgxact_access_AtEOXact.v2.patch | application/octet-stream | 3.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David Christensen | 2017-02-20 17:22:48 | Re: [PATCH] Add pg_disable_checksums() and supporting infrastructure |
Previous Message | Robert Haas | 2017-02-20 17:12:55 | Re: Replication vs. float timestamps is a disaster |