On 2 September 2016 at 16:50, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> So here's a simpler patch that just dirties the slot when it's
>> replayed something from it on the SQL interface [...]
>
> Okay that sounds reasonable, the SQL interface is already somewhat different
> than walsender as it does not really "stream" so makes sense to improve the
> behavior there.
Yep. For the walsender interface it's reasonable to expect the
downstream to keep track of its progress - even if right now
pg_recvlogical doesn't do so very well. It's not practical for the SQL
interface.
> As a side note, I would really like to have cursor-like SQL
> interface which behaves more like walsender one but that's very different
> patch.
Me too. Specifically, I'd really like to be able to "peek, confirm,
peek, confirm" and also be able to specify my own start lsn for peek.
But as you say, that's a different patch.
>> The alternative is probably to add a second, "softer" dirty tracking
>> method that only causes a write at a clean shutdown or forced
>> checkpoint - and maybe doesn't bother fsync()ing. That's a bit more
>> invasive but would work for walsender use as well as the SQL
>> interface. I don't think it's worth the bother, since in the end
>> callers have to be prepared for repeated data on crash anyway.
>>
>
> Correct me if I am wrong but I think the only situation where it would
> matter is on server that restarts often or crashes often (as the logical
> decoding then has to do the work many times) but I don't think it's worth
> optimizing for that kind of scenario.
Right. Though it's not so much doing the work many times that's the
concern - this change doesn't affect restart_lsn at all. It's that it
sends already-seen-and-confirmed changes to the output plugin and
therefore the client.
Not that big a deal. As far as I'm concerned, it's the job of users of
the walsender interface to keep track of the position they've consumed
up to and specify it to subsequent START_REPLICATION calls. You can't
do that on the SQL interface, which is why this change is needed
there.
Maybe the docs should be more explicit about the reason to specify
start lsn to START_REPLICATION not just 0/0 though, and also that we
will disregard it if it's less than the stored confirmed_flush_lsn.
Again, separate patch.
So the main change becomes the one-liner in my prior mail.
-- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services