Re: Changed behavior in rewriteheap

From: Erik Nordström <erik(at)timescale(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Changed behavior in rewriteheap
Date: 2024-11-22 15:02:13
Message-ID: CACAa4V+Vbe1BWWnssu30nMBvi8tShn41e_cs-PtUpmbEvFE-eQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 22, 2024 at 3:53 PM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

> On 22/11/2024 15:56, Erik Nordström wrote:
> > Yes, it solves the issue so it looks good.
> >
> > Just a minor nit: the code uses both blokno as local variable for
> > pending_writes[i].blkno and directly accessing pending_writes[i].blkno.
> > Maybe it is better to just use the local variable. For example, change
> >
> > ++ b/src/backend/storage/smgr/bulk_write.c
> > @@ -304,7 +304,8 @@ smgr_bulk_flush(BulkWriteState *bulkstate)
> > }
> >
> > smgrextend(bulkstate->smgr, bulkstate->forknum,
> > blkno, page, true);
> > - bulkstate->relsize = pending_writes[i].blkno + 1;
> > + bulkstate->relsize++;
> > + Assert(bulkstate->relsize == blkno + 1);
> >
> > Just a suggestion.
>
> Made that change and committed to master and REL_17_STABLE. I didn't
> bother with the assertion though. Also I removed the 'pages_written'
> field, it was not used for anything anymore.
>

Sounds good. Thank you again!

-Erik

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Anthonin Bonnefoy 2024-11-22 15:30:45 Re: Consider pipeline implicit transaction as a transaction block
Previous Message Heikki Linnakangas 2024-11-22 14:53:00 Re: Changed behavior in rewriteheap