On 2015-02-10 22:06:34 +0900, Michael Paquier wrote: > On Tue, Feb 10, 2015 at 9:46 PM, Andres Freund <andres@2ndquadrant.com> > wrote: > > > Yea, it really looks like the above commit is to "blame". The new xmin > > tracking infrastructure doesn't know about the historical snapshot... > > > > I think that we need a better regression coverage here... For example, we > could add some tap tests in test_decoding to test streaming of logical > changes. This would help in the future to detect such problems via the > buildfarm.
I agree. It's more or less a accident that the assert - which just should be moved in the regd_count == 0 branch - didn't trigger for the SQL interface. The snapshot acquired by the SELECT statement prevents it there.
It's not entirely trivial to add tests for receivelogical though. You need to stop it programatically after a while.
I reckon we should improve that then.
What about an idle timeout for pg_recvlogical, so we can get it to time out after (say) 5 seconds of no data? Time-based facilities aren't great in tests though.
The SQL interface can stop after a given number of changes received or a target LSN. Is there a practical reason pg_recvlogical doesn't? Or just that there wasn't a reason to implement it? I figured I'd ask before jumping in and trying to add it in case I'd be wasting my time trying.