Re: define PG_REPLSLOT_DIR

Alvaro Herrera <alvherre@alvh.no-ip.org>

From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2024-08-16T00:40:42Z
Lists: pgsql-hackers
On 2024-Aug-14, Bertrand Drouvot wrote:

> Out of curiosity, checking the sizes of affected files (O2, no debug):
> 
> with patch:
> 
>    text    data     bss     dec     hex filename
>   30304       0       8   30312    7668 src/backend/replication/logical/reorderbuffer.o

> without patch:
>   30286       0       8   30294    7656 src/backend/replication/logical/reorderbuffer.o

Hmm, I suppose this delta can be explained because because the literal
string is used inside larger snprintf() format strings or similar, so
things like

            snprintf(path, sizeof(path),
-                    "pg_replslot/%s/%s", slotname,
+                    "%s/%s/%s", PG_REPLSLOT_DIR, slotname,
                     spill_de->d_name);

and

                ereport(ERROR,
                        (errcode_for_file_access(),
-                        errmsg("could not remove file \"%s\" during removal of pg_replslot/%s/xid*: %m",
-                               path, slotname)));
+                        errmsg("could not remove file \"%s\" during removal of %s/%s/xid*: %m",
+                               PG_REPLSLOT_DIR, path, slotname)));

I don't disagree with making this change, but changing some of the other
directory names you suggest, such as

> pg_notify
> pg_serial
> pg_subtrans
> pg_multixact
> pg_wal

would probably make no difference, since there are no literal strings
that contain that as a substring(*).  It might some sense to handle
pg_tblspc similarly.  As for pg_logical, I think you'll want separate
defines for pg_logical/mappings, pg_logical/snapshots and so on.


(*) I hope you're not going to suggest this kind of change (word-diff):

    if (GetOldestSnapshot())
        ereport(ERROR,
                (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                 errmsg("[-pg_wal-]{+%s+}_replay_wait() must be only called without an active or registered snapshot"{+, PG_WAL_DIR+}),
                 errdetail("Make sure [-pg_wal-]{+%s+}_replay_wait() isn't called within a transaction with an isolation level higher than READ COMMITTED, another procedure, or a function."{+, PG_WAL_DIR+})));

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Los trabajadores menos efectivos son sistematicamente llevados al lugar
donde pueden hacer el menor daño posible: gerencia."  (El principio Dilbert)



Commits

  1. Define PG_TBLSPC_DIR for path pg_tblspc/ in data folder

  2. Define PG_LOGICAL_DIR for path pg_logical/ in data folder

  3. Define PG_REPLSLOT_DIR for path pg_replslot/ in data folder