On 2016-04-06 14:30:21 +0100, Simon Riggs wrote: > On 6 April 2016 at 14:15, Craig Ringer <cr...@2ndquadrant.com> wrote: > ... > > Nice summary > > Failover slots are optional. And they work on master. > > While the other approach could also work, it will work later and still > require a slot on the master. > > > => I don't see why having Failover Slots in 9.6 would prevent us from > having something else later, if someone else writes it. > > > We don't need to add this to core. Each plugin can independently write is > own failover code. Works, but doesn't seem like the right approach for open > source. > > > => I think we should add Failover Slots to 9.6.
Simon, please don't take this personal; because of the other ongoing thread. I don't think this is commit-ready. For one I think this is architecturally the wrong choice. But even leaving that fact aside, and considering this a temporary solution (we can't easily remove), there appears to have been very little code level review (one early from Petr in [1], two by Oleksii mostly focusing on error messages [2] [3]). The whole patch was submitted late to the 9.6 cycle. Quickly skimming 0001 in [4] there appear to be a number of issues: * LWLockHeldByMe() is only for debugging, not functional differences * ReplicationSlotPersistentData is now in an xlog related header * The code and behaviour around name conflicts of slots seems pretty raw, and not discussed * Taking spinlocks dependant on InRecovery() seems like a seriously bad idea * I doubt that the archive based switches around StartupReplicationSlots do what they intend. Afaics that'll not work correctly for basebackups taken with -X, without recovery.conf That's from a ~5 minute skim, of one patch in the series. [1] http://archives.postgresql.org/message-id/CALLjQTSCHvcsF6y7%3DZhmdMjJUMGLqt1-6Pz2rtb7PfFLxFfBOw%40mail.gmail.com [2] http://archives.postgresql.org/message-id/FA68178E-F0D1-47F6-9791-8A3E2136C119%40hintbits.com [3] http://archives.postgresql.org/message-id/9B503EB5-676A-4258-9F78-27FC583713FE%40hintbits.com [4] http://archives.postgresql.org/message-id/camsr+ye6lny2e0tbuaqb+ntvb6w-dhjaflq0-zbal7g7hjh...@mail.gmail.com Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
