Re: Stopping logical replication protocol

<div dir="ltr">> <span style="font-size:12.8px">Vladimir, can I have your opinion on the latest patch or if you want
tomake changes, an updated patch?</span><p style="font-size:12.8px">I am satisfied with all our changes and I thinks it
enoughto complete this PR.</div><div class="gmail_extra"><br /><div class="gmail_quote">2016-10-03 6:51 GMT+03:00 Craig
Ringer<span dir="ltr"><<a href="mailto:craig@2ndquadrant.com"
target="_blank">craig@2ndquadrant.com</a>></span>:<br/><blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px#ccc solid;padding-left:1ex"><div dir="ltr"><span class=""><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 /> > On Tue, Sep 27, 2016 at 11:05 AM, Craig
Ringer<<a href="mailto:craig@2ndquadrant.com" target="_blank">craig@2ndquadrant.com</a>> wrote:<br /> > >
On26 September 2016 at 21:52, Vladimir Gordiychuk <<a href="mailto:folyga@gmail.com"
target="_blank">folyga@gmail.com</a>>wrote:<br /> > >>>You should rely on time I tests as little as
possible.Some of the test<br /> > >>> hosts are VERY slow. If possible something deterministic 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 /> > >> In that case we can say, before
thischanges stopping logical replication<br /> > >> gets N ms but after apply changes it gets NN ms where NN
msless 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 I added 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
leftdiag 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'tcurrently have any automated benchmarking infrastructure.<br /> ><br /> > Which seems like this patch is not
completeyet.</span><p dir="ltr">Personally I think it is. I'm just explaining why I adjusted Vladimir's tests to be
lesstiming sensitive and rely on a qualitative property instead.<p dir="ltr">That said, it's had recent change and it
isn'ta big intrusive change that really need attention this cf.<br /><span class=""><p dir="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
withupdated tests shows up.</span><p dir="ltr">I'll move it now. I think the tests are fine, if Vladimir agrees, so IMO
it'sready to go. More eyes wouldn't hurt though.<br /><p dir="ltr">If Vladimir wants benchmarking based tests that's a
wholeseparate project IMO. Something that will work robustly on the weird slow machines we have isn't simple. Probably
anew buildfarm option etc since we won't want to clutter the really slow old niche boxes with it.<p dir="ltr">Vladimir,
canI have your opinion on the latest patch or if you want to make changes, an updated patch?<br /><p dir="ltr"><br
/></div></blockquote></div><br/></div>