On 05/09/16 04:41, Craig Ringer wrote:
> On 2 September 2016 at 17:49, Craig Ringer <craig@2ndquadrant.com> wrote:
>
>> So the main change becomes the one-liner in my prior mail.
>
> Per feedback from Simon, updated with a new test in src/test/recovery .
>
> If you revert the change to
> src/backend/replication/logical/logicalfuncs.c then the test will
> start failing.
>
> I'd quite like to backpatch the fix since it's simple and safe, but I
> don't feel that it's hugely important to do so. This is an annoyance
> not a serious data risking bug.
>
> My only concern here is that we still lose position on crash. So
> SQL-interface callers should still be able to cope with it. But they
> won't see it happen if they're only testing with normal shutdowns,
> host reboots, etc. In practice users aren't likely to notice this
> anyway, though, since most people don't restart the server all the
> time. I think it's better than what we have.
>
> This issue could be eliminated completely by calling
> ReplicationSlotSave() after ReplicationSlotMarkDirty(). This would
> force an immediate flush after a non-peeked SQL interface decoding
> call. It means more fsync()ing, though, and the SQL interface can only
> be used by polling so that could be a significant performance hit.
> We'd want to only flush when the position changed, but even then it'll
> mean a sync every time anything gets returned.
>
> The better alternative is to add a variant on
> pg_logical_slot_get_changes(...) etc that accepts a start LSN. But
> it's not convenient or easy for SQL callers to extract the last commit
> LSN received from the last call to pass it to the next one, so this
> isn't simple, and is probably best tackled as part of the SQL
> interface API change Petr and Andres discussed in this thread.
>
It isn't? I thought lsn is first column of the output of that function?
-- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services