Re: Rework the way multixact truncations work

On 2015-06-29 23:54:40 +1200, Thomas Munro wrote:
> On Mon, Jun 22, 2015 at 7:24 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > It'd be very welcome to see some wider testing and review on this.
>
> I started looking at this and doing some testing. Here is some
> initial feedback:
>
> Perhaps vac_truncate_clog needs a new name now that it does more,
> maybe vac_truncate_transaction_logs?

It has done more before, so I don't really see a connection to this
patch...

> MultiXactState->sawTruncationCkptCyle: is 'Cyle' supposed to be 'Cycle'?

Oops.

> In the struct xl_multixact_truncate, and also the function
> WriteMTruncateXlogRec and other places, I think you have confused the
> typedefs MultiXactOffset and MultiXactId. If I'm not mistaken,
> startMemb and endMemb have the correct type, but startOff and endOff
> should be of type MultiXactId despite their names (the *values* stored
> inside pg_multixact/offsets are indeed offsets (into
> pg_multixact/members), but their *location* is what a multixact ID
> represents).

IIRC I did it that way to make clear this is just 'byte' type offsets,
without other meaning. Wasn't a good idea.

> I was trying to understand if there could be any problem caused by
> setting latest_page_number to the pageno that holds (or will hold)
> xlrec.endOff in multixact_redo. The only thing that jumps out at me
> is that the next call to SlruSelectLRUPage will no longer be prevented
> from evicting the *real* latest page (the most recently created page).

That hasn't changed unless I miss something?

> In SlruDeleteSegment, is it OK to call unlink() while holding the SLRU
> control lock?

I think it's safer than not doing it, but don't particularly care.

> In find_multixact_start you call SimpleLruFlush before calling
> SimpleLruDoesPhysicalPageExist. Should we do something like this
> instead? https://gist.github.com/macdice/8e5b2f0fe3827fdf3d5a

I'm currently slightly inclined to do it "my way". They way these
functions are used it doesn't seem like a bad property to ensure things
are on disk.

> I think saw some extra autovacuum activity that I didn't expect in a
> simple scenario, but I'm not sure and will continue looking tomorrow.

Cool, thanks!

Greetings,

Andres Freund