Hi,
On Tue, Aug 20, 2024 at 05:47:57PM +0900, Michael Paquier wrote:
> On Mon, Aug 19, 2024 at 02:11:55PM +0000, Bertrand Drouvot wrote:
> > I made the changes for pg_tblspc in pg_combinebackup.c as the number of occurences
> > are greater that the "pg_wal" ones and we were to define PG_TBLSPC_DIR in any
> > case.
> >
> > Please find attached the related patches.
>
> No real objection about the replslot and pg_logical bits.
Thanks for looking at it!
>
> - * $PGDATA/pg_tblspc/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber
> + * $PGDATA/PG_TBLSPC_DIR/spcoid/PG_MAJORVER_CATVER/dboid/relfilenumber
>
> For the tablespace parts, I am not sure that I would update the
> comments to reflect the variables, TBH. Somebody reading the comments
> would need to refer back to pg_tblspc/ in the header.
I'm not sure as, for example, for PG_STAT_TMP_DIR we have those ones:
src/backend/backup/basebackup.c: * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped
src/bin/pg_rewind/filemap.c: * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped
so I thought it would be better to be consistent.
That said, I don't have a strong opinion about it, but then I guess you'd want to
do the same for the ones related to replslot:
src/backend/replication/slot.c: * Each replication slot gets its own directory inside the $PGDATA/PG_REPLSLOT_DIR
src/backend/utils/adt/genfile.c: * Function to return the list of files in the PG_REPLSLOT_DIR/<replication_slot>
and pg_logical:
src/backend/utils/adt/genfile.c: * Function to return the list of files in the PG_LOGICAL_SNAPSHOTS_DIR directory.
src/backend/utils/adt/genfile.c: * Function to return the list of files in the PG_LOGICAL_MAPPINGS_DIR directory.
, right?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
