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.
Also, maybe naming the first variable cachedFetchSubXid would make it a bit
clearer?
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.