| From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
|---|---|
| To: | shihao zhong <zhong950419(at)gmail(dot)com> |
| Cc: | Peipei YIN <yinpeipei0426(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: [PATCH] Refactor remaining zero-fill relation extensions to use smgrzeroextend() |
| Date: | 2026-09-25 22:41:06 |
| Message-ID: | CALj2ACWwRreb=Tai_6yFvwA_YOtKBje3zcFTf2FZTpVe1iM_bQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Thu, Sep 24, 2026 at 10:07 PM shihao zhong <zhong950419(at)gmail(dot)com> wrote:
>
> I reviewed v2. It fixes both regressions I raised on v1. With one
> block, mdzeroextend() takes the same FileZero() path as the old
> mdextend() call, so nothing changes at runtime there.
Thanks for reviewing.
> One request. smgrzeroextend(..., nblocks - 1, 1, ...) looks like a bug
> unless you know why, and changing it to (0, nblocks) brings the 2x
> regression back. Please add a one line comment in bufmgr.c and md.c
> saying only the last block is written on purpose.
It can take a while to understand that part when first looking at it.
Still, I would prefer not to add the comment, since it would mostly
end up restating what the code already does, unless anyone thinks
otherwise.
> Nit, the "don't set checksum" comment in bulk_write.c no longer
> applies, since no page is passed.
Reworded it to match an existing comment around smgrzeroextend().
> Note that v2 does not move toward the zero page detection idea. The
> blocks before the last one are still holes, which read as zeros and
> never pass through smgrzeroextend(). Making them non-zero means writing
> them, which is the 2x cost Bharath measured.
Right, I don't think this patch was meant to solve that, though I may
be missing something. What it does is keep the responsibilities of
smgrextend() and smgrzeroextend() separate, as mentioned upthread, and
that alone is worth having IMHO. I think the zero page detection part
would need changes to the API itself. Peipei, appreciate any thoughts
on this.
I attached the v3 patch. Please have a look. I don't think this needs
to be back-patched, since it is not fixing a bug.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Use-smgrzeroextend-in-a-few-more-places.patch | application/x-patch | 4.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shihao zhong | 2026-09-25 22:49:07 | Re: REPACK (CONCURRENTLY) can't complete after ~105M concurrent updates/deletes |
| Previous Message | shihao zhong | 2026-09-25 22:32:30 | Re: REPACK (CONCURRENTLY) can lose data in pg_dump output |