Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...) - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)
Date
Msg-id 20200622205712.kgnxpkrebc4g2con@alap3.anarazel.de
Whole thread Raw
In response to Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)  (Justin Pryzby <pryzby@telsasoft.com>)
Responses Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)
Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)
List pgsql-hackers
Hi,

On 2020-06-22 15:43:11 -0500, Justin Pryzby wrote:
> On Mon, Jun 22, 2020 at 01:09:39PM -0700, Andres Freund wrote:
> > I'm also uncomfortable with the approach of just copying all of
> > LVRelStats in several places:
> > 
> > >  /*
> > > @@ -1580,9 +1648,15 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
> > >      int            uncnt = 0;
> > >      TransactionId visibility_cutoff_xid;
> > >      bool        all_frozen;
> > > +    LVRelStats    olderrinfo;
> 
> I guess the alternative is to write like
> 
> LVRelStats    olderrinfo = {
>     .phase = vacrelstats.phase,
>     .blkno = vacrelstats.blkno,
>     .indname = vacrelstats.indname,
> };

No, I don't think that's a solution. I think it's wrong to have
something like olderrinfo in the first place. Using a struct with ~25
members to store the current state of three variables just doesn't make
sense.  Why isn't this just a LVSavedPosition struct or something like
that?

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: [HACKERS] Custom compression methods
Next
From: Thomas Munro
Date:
Subject: Re: Parallel Seq Scan vs kernel read ahead