Re: define PG_REPLSLOT_DIR

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