Re: WIP: Failover Slots - Mailing list pgsql-hackers
| From | Craig Ringer |
|---|---|
| Subject | Re: WIP: Failover Slots |
| Date | |
| Msg-id | CAMsr+YFsAbkfqDQ=qQi4pYARfoa5MZ1zgDRSEkH-oOR5Eh=WxA@mail.gmail.com Whole thread Raw |
| In response to | WIP: Failover Slots (Simon Riggs <simon@2ndQuadrant.com>) |
| Responses |
Re: WIP: Failover Slots
(Craig Ringer <craig@2ndquadrant.com>)
|
| List | pgsql-hackers |
On 2 January 2016 at 08:50, Simon Riggs <simon@2ndquadrant.com> wrote:
Patch is WIP, posted for comment, so you can see where I'm going.
At present the patch does not appear to work. No slots are visible in the replica's pg_replication_slots before or after promotion and no slot information is written to the xlog according to pg_xlogdump:
$ ~/pg/96/bin/pg_xlogdump -r ReplicationSlot 000000010000000000000001 000000010000000000000003
pg_xlogdump: FATAL: error in WAL record at 0/301DDE0: invalid record length at 0/301DE10
so it's very much a WIP. I've read through it and think the idea makes sense, it's just still missing some pieces...
So. Initial review comments.
This looks pretty incomplete:
+ if (found_duplicate)
+ {
+ LWLockRelease(ReplicationSlotAllocationLock);
+
+ /*
+ * Do something nasty to the sinful duplicants, but
+ * take with locking.
+ */
+
+ LWLockAcquire(ReplicationSlotAllocationLock, LW_SHARED);
+ }
... and I'm not sure I understand how the possibility of a duplicate slot can arise in the first place, since you cannot create a slot on a read replica. This seems unnecessary.
I'm not sure I understand why, in ReplicationSlotRedoCreate, it's especially desirable to prevent blocking iteration over pg_replication_slots or acquiring a slot. When redoing a slot create isn't that exactly what we should do? This looks like it's been copied & pasted verbatim from CheckPointReplicationSlots . There it makes sense, since the slots may be in active use. During redo it seems reasonable to just acquire ReplicationSlotControlLock.
I'm not a fan of the ReplicationSlotInWAL typedef for ReplicationSlotPersistentData. Especially as it's only used in the redo functions but *not* when xlog is written out. I'd like to just replace it.
Purely for convenient testing there's a shell script in the tree - https://github.com/2ndQuadrant/postgres/blob/dev/failover-slots/failover-slot-test.sh . Assuming a patched 9.6 in $HOME/pg/96 it'll do a run-through of the patch. I'll attempt to convert this to use the new test infrastructure, but needed something ASAP for development. Posted in case it's useful to others.
Now it's time to look into why this doesn't seem to be generating any xlog when by rights it seems like it should. Also into at what point exactly we purge existing slots on start / promotion of a read-replica.
TL;DR: this doesn't work yet, working on it.
pgsql-hackers by date: