From: | Simon Riggs <simon(dot)riggs(at)enterprisedb(dot)com> |
---|---|
To: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
Cc: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Pengchengliu <pengchengliu(at)tju(dot)edu(dot)cn>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: suboverflowed subtransactions concurrency performance optimize |
Date: | 2022-03-07 13:27:40 |
Message-ID: | CANbhV-E9eFXNuN-NMnH5xjRDGAM+Vi8+CUi5c3cXgeo93NM9Aw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, 7 Mar 2022 at 09:49, Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Mon, Jan 17, 2022 at 01:44:02PM +0000, Simon Riggs wrote:
> >
> > Re-attached, so that the CFapp isn't confused between the multiple
> > patches on this thread.
>
> Thanks a lot for working on this!
>
> The patch is simple and overall looks good to me. A few comments though:
>
>
> +/*
> + * Single-item cache for results of SubTransGetTopmostTransaction. It's worth having
> + * such a cache because we frequently find ourselves repeatedly checking the
> + * same XID, for example when scanning a table just after a bulk insert,
> + * update, or delete.
> + */
> +static TransactionId cachedFetchXid = InvalidTransactionId;
> +static TransactionId cachedFetchTopmostXid = InvalidTransactionId;
>
> The comment is above the 80 chars after
> s/TransactionLogFetch/SubTransGetTopmostTransaction/, and I don't think this
> comment is valid for subtrans.c.
What aspect makes it invalid? The comment seems exactly applicable to
me; Andrey thinks so also.
> Also, maybe naming the first variable cachedFetchSubXid would make it a bit
> clearer?
Sure, that can be done.
> It would be nice to see some benchmarks, for both when this change is
> enough to avoid a contention (when there's a single long-running overflowed
> backend) and when it's not enough. That will also be useful if/when working on
> the "rethink_subtrans" patch.
The patch doesn't do anything about the case of when there's a single
long-running overflowed backend, nor does it claim that.
The patch will speed up calls to SubTransGetTopmostTransaction(), which occur in
src/backend/access/heap/heapam.c
src/backend/utils/time/snapmgr.c
src/backend/storage/lmgr/lmgr.c
src/backend/storage/ipc/procarray.c
The patch was posted because TransactionLogFetch() has a cache, yet
SubTransGetTopmostTransaction() does not, yet the argument should be
identical in both cases.
--
Simon Riggs http://www.EnterpriseDB.com/
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2022-03-07 13:56:24 | Re: [RFC] building postgres with meson |
Previous Message | Justin Pryzby | 2022-03-07 13:09:22 | Re: Add parameter jit_warn_above_fraction |