Assorted fixes related to WAL files (was: Use XLogFromFileName() in pg_resetwal to parse position from WAL file)

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Assorted fixes related to WAL files (was: Use XLogFromFileName() in pg_resetwal to parse position from WAL file)
Date: 2022-10-04 07:50:54
Message-ID: CALj2ACW1o_aHG0AraxQ2ys9SjUudSAJTqqOX=imGkrBbWuts6g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Oct 4, 2022 at 12:11 PM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> >
> > > static uint32 minXlogTli = 0;
>
> I have found other three instances of this in xlog.c and
> pg_receivewal.c. Do they worth fixing?
>
> (pg_upgarade.c has "uint32 tli/logid/segno but I'm not sure they need
> to be "fixed". At least the segno is not a XLogSegNo.)

There are quite a number of places where data types need to be fixed,
see XLogFileNameById() callers. They are all being parsed as uint32
and then used. I'm not sure if we want to fix all of them.

I think I found that we can fix/refactor few WAL file related things:

1. 0001 replaces explicit WAL file parsing code with
XLogFromFileName() and uses XLByteToSeg() in pg_resetwal.c. This was
not done then (in PG 10) because the XLogFromFileName() wasn't
accepting file size as an input parameter and pg_resetwal needed to
use WAL file size from the controlfile. Thanks to the commit
fc49e24fa69a15efacd5b8958115ed9c43c48f9a which added the
wal_segsz_bytes parameter to XLogFromFileName(). This removes using
extra variables in pg_resetwal.c and a bit of duplicate code too. It
also replaces the explicit code with the XLByteToSeg() macro.

2. 0002 replaces MAXPGPATH with MAXFNAMELEN for WAL file names.
MAXFNAMELEN (64 bytes) is typically meant to be used for all WAL file
names across the code base. Because the WAL file names in postgres
can't be bigger than 64 bytes, in fact, not more than XLOG_FNAME_LEN
(24 bytes) but there are suffixes, timeline history files etc. To
accommodate all of that MAXFNAMELEN is introduced. There are some
places in the code base that still use MAXPGPATH (1024 bytes) for WAL
file names which is an unnecessary wastage of stack memory. This makes
code consistent across and saves a bit of space.

3. 0003 replaces WAL file name calculation with XLogFileNameById() in
pg_upgrade/controldata.c to be consistent across the code base. Note
that this requires us to change the nextxlogfile size from hard-coded
25 bytes to MAXFNAMELEN (64 bytes).

I'm attaching the v2 patch set.

Thoughts?

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachment Content-Type Size
v2-0001-Use-WAL-file-related-inline-functions-macros-in-p.patch application/octet-stream 2.5 KB
v2-0002-Use-MAXFNAMELEN-for-WAL-file-names-instead-of-MAX.patch application/octet-stream 3.1 KB
v2-0003-Use-XLogFileNameById-in-pg_upgrade-controldata.c.patch application/octet-stream 1.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2022-10-04 08:05:40 Re: installcheck-world concurrency issues
Previous Message Peter Eisentraut 2022-10-04 07:41:19 future of serial and identity columns