Re: cleaning up a few CLOG-related things

Heikki Linnakangas <hlinnaka@iki.fi>

From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Robert Haas <robertmhaas@gmail.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2021-01-25T19:11:51Z
Lists: pgsql-hackers
On 25/01/2021 18:56, Robert Haas wrote:
> I attach a series of proposed patches to slightly improve some minor
> things related to the CLOG code.
> 
> [patches 0001 - 0003]

Makes sense.

> 0004 - In StartupCLOG(), correct an off-by-one error. Currently, if
> the nextXid is exactly a multiple of the number of CLOG entries that
> fit on a page, then the value we compute for
> XactCtl->shared->latest_page_number is higher than it should be by 1.
> That's because nextXid represents the first XID that hasn't yet been
> allocated, not the last one that gets allocated.

Yes.

> So, the CLOG page gets created when nextXid advances from the first
> value on the page to the second value on the page, not when it
> advances from the last value on the previous page to the first value
> on the current page.
Yes. It took me a moment to understand that explanation, though. I'd 
phrase it something like "nextXid is the next XID that will be used, but 
we want to set latest_page_number according to the last XID that's 
already been used. So retreat by one."

Having a separate FullTransactionIdToLatestPageNumber() function for 
this seems like overkill to me.

> Some of the other SLRUs have similar issues as a result of
> copy-and-paste work over the years. I plan to look at tidying that
> stuff up, too. However, I wanted to post (and probably commit) these
> patches first, partly to get some feedback, and also because all the
> cases are a little different and I want to make sure to do a proper
> analysis of each one.

Yeah, multixact seems similar at least.

- Heikki



Commits

  1. In TrimCLOG(), don't reset XactCtl->shared->latest_page_number.

  2. In clog_redo(), don't set XactCtl->shared->latest_page_number.

  3. Move StartupCLOG() calls to just after we initialize ShmemVariableCache.