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 13:56:36
Message-ID: CACAa4V+G3-1dZV7-VnaGE0MwOGA-+piZkus_xj9vjGNO7r9BRQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

> On 22/11/2024 15:02, Matthias van de Meent wrote:
> > I think I'd go with a patch like attached, where the bulk writer
> > registers that it started with .relsize pages in the relfork, and use
> > that for smgrextend() decisions. It now also records pages_written as
> > a separate but accurate value.
>
> Looks good to me. Eric, can you confirm that Matthias's patch fixes the
> problem for you?
>
>
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.

Thanks for the quick action!

-Erik

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2024-11-22 14:11:55 Re: Replace current implementations in crypt() and gen_salt() to OpenSSL
Previous Message Pavel Luzanov 2024-11-22 13:53:32 Re: psql: Fix \pset by adding missed xheader_width option to the help