Move replication slot structures/enums/macros to a new header file for better usability by external tools/modules

On Mon, Feb 7, 2022 at 4:22 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

While working on pg_replslotdata tool [1], it was observed that some
of the replication slot structures/enums/macros such as
ReplicationSlotPersistentData, ReplicationSlotPersistency,
ReplicationSlotOnDisk, ReplicationSlotOnDiskXXXX etc. are currently in
replication/slot.h and replication/slot.c. This makes the replication
slot on disk data structures unusable by the external tools/modules.
How about moving these structures to a new header file called
slot_common.h as attached in the patch here?

Thoughts?

PS: I'm planning to have the tool separately, as it was rejected to be in core.

[1] /messages/by-id/CALj2ACW0rV5gWK8A3m6_X62qH+Vfaq5hznC=i0R5Wojt5+yhyw@mail.gmail.com

Regards,
Bharath Rupireddy.

Recently I was also looking to add some new enums but I found it
was difficult to find any good place to put them where they could be
shared by the replication code and the pg_recvlogical tool.

So +1 to your suggestion to have a common header, but I wonder can it
have a more generic name (e.g. repl_common.h? ...) since the
stuff I wanted to put there was not really "slot" related.

------
Kind Regards,
Peter Smith.
Fujitsu Australia.

On Wed, Feb 9, 2022 at 2:16 AM Peter Smith <smithpb2250@gmail.com> wrote:

On Mon, Feb 7, 2022 at 4:22 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:

Hi,

While working on pg_replslotdata tool [1], it was observed that some
of the replication slot structures/enums/macros such as
ReplicationSlotPersistentData, ReplicationSlotPersistency,
ReplicationSlotOnDisk, ReplicationSlotOnDiskXXXX etc. are currently in
replication/slot.h and replication/slot.c. This makes the replication
slot on disk data structures unusable by the external tools/modules.
How about moving these structures to a new header file called
slot_common.h as attached in the patch here?

Thoughts?

PS: I'm planning to have the tool separately, as it was rejected to be in core.

[1] /messages/by-id/CALj2ACW0rV5gWK8A3m6_X62qH+Vfaq5hznC=i0R5Wojt5+yhyw@mail.gmail.com

Recently I was also looking to add some new enums but I found it
was difficult to find any good place to put them where they could be
shared by the replication code and the pg_recvlogical tool.

So +1 to your suggestion to have a common header, but I wonder can it
have a more generic name (e.g. repl_common.h? ...) since the
stuff I wanted to put there was not really "slot" related.

Thanks. repl_common.h sounds cool and generic IMO too, so I changed
it. Another important note here is to let this file have only
replication data structures and functions that are meant/supposed to
be usable across entire postgres modules - core, tools, contrib
modules, the internal data structures can be added elsewhere.

Attaching v2 patch.

I will add a CF entry a day or two later.

Regards,
Bharath Rupireddy.

Hi,

On 2022-02-07 10:52:08 +0530, Bharath Rupireddy wrote:

While working on pg_replslotdata tool [1], it was observed that some
of the replication slot structures/enums/macros such as
ReplicationSlotPersistentData, ReplicationSlotPersistency,
ReplicationSlotOnDisk, ReplicationSlotOnDiskXXXX etc. are currently in
replication/slot.h and replication/slot.c. This makes the replication
slot on disk data structures unusable by the external tools/modules.

FWIW, I still don't see a point in pg_replslotdata. And I don't think these
datastructures should ever be accessed outside the server environment.

Greetings,

Andres Freund