Re: Stopping logical replication protocol

<div dir="ltr"><p dir="ltr"><p dir="ltr">On 3 Oct. 2016 10:15, "Michael Paquier" <<a
href="mailto:michael.paquier@gmail.com"target="_blank">michael.paquier@gmail.com</a>> wrote:<br /> ><br /> >
OnTue, Sep 27, 2016 at 11:05 AM, Craig Ringer <<a href="mailto:craig@2ndquadrant.com"
target="_blank">craig@2ndquadrant.com</a>>wrote:<br /> > > On 26 September 2016 at 21:52, Vladimir Gordiychuk
<<ahref="mailto:folyga@gmail.com" target="_blank">folyga@gmail.com</a>> wrote:<br /> > >>>You should
relyon time I tests as little as possible. Some of the test<br /> > >>> hosts are VERY slow. If possible
somethingdeterministic should be used.<br /> > >><br /> > >> That's why this changes difficult to
verify.Maybe in that case we should<br /> > >> write benchmark but not integration test?<br /> > >>
Inthat case we can say, before this changes stopping logical replication<br /> > >> gets N ms but after apply
changesit gets NN ms where NN ms less than N ms.<br /> > >> Is it available add this kind of benchmark to
postgresql?I will be grateful<br /> > >> for links.<br /> > ><br /> > > It's for that reason that
Iadded a message printed only in verbose<br /> > > mode that pg_recvlogical emits when it's exiting after a<br />
>> client-initiated copydone.<br /> > ><br /> > > You can use the TAP tests, print diag messages,
etc.But we generally<br /> > > want them to run fairly quickly, so big benchmark runs aren't<br /> > >
desirable.You'll notice that I left diag messages in to report the<br /> > > timing for the results in your
tests,I just changed the tests so they<br /> > > didn't depend on very tight timing for success/failure
anymore.<br/> > ><br /> > > We don't currently have any automated benchmarking infrastructure.<br />
><br/> > Which seems like this patch is not complete yet.<p dir="ltr">Personally I think it is. I'm just
explainingwhy I adjusted Vladimir's tests to be less timing sensitive and rely on a qualitative property instead.<p
dir="ltr">Thatsaid, it's had recent change and it isn't a big intrusive change that really need attention this cf.<br
/><pdir="ltr">>  I am marking it as<br /> > returned with feedback, but it may be a better idea to move it to
next<br/> > CF once a new version with updated tests shows up.<p dir="ltr">I'll move it now. I think the tests are
fine,if Vladimir agrees, so IMO it's ready to go. More eyes wouldn't hurt though.<br /><p dir="ltr">If Vladimir wants
benchmarkingbased tests that's a whole separate project IMO. Something that will work robustly on the weird slow
machineswe have isn't simple. Probably a new buildfarm option etc since we won't want to clutter the really slow old
nicheboxes with it.<p dir="ltr">Vladimir, can I have your opinion on the latest patch or if you want to make changes,
anupdated patch?<br /><p dir="ltr"><br /></div>