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
