On 20 November 2015 at 01:48, Bert Huijben <[email protected]> wrote: >> -----Original Message----- >> From: Bert Huijben [mailto:[email protected]] >> Sent: donderdag 19 november 2015 23:41 >> To: 'Ivan Zhakov' <[email protected]> >> Cc: [email protected] >> Subject: RE: svn commit: r1715228 - >> /subversion/trunk/subversion/libsvn_ra_serf/util.c >> >> >> >> > -----Original Message----- >> > From: Ivan Zhakov [mailto:[email protected]] >> > Sent: donderdag 19 november 2015 23:12 >> > To: Bert Huijben <[email protected]> >> > Cc: [email protected] >> > Subject: Re: svn commit: r1715228 - >> > /subversion/trunk/subversion/libsvn_ra_serf/util.c >> > >> > On 20 November 2015 at 00:03, Bert Huijben <[email protected]> wrote: >> > >> -----Original Message----- >> > >> From: [email protected] [mailto:[email protected]] >> > >> Sent: donderdag 19 november 2015 19:03 >> > >> To: [email protected] >> > >> Subject: svn commit: r1715228 - >> > >> /subversion/trunk/subversion/libsvn_ra_serf/util.c >> > >> >> > >> Author: rhuijben >> > >> Date: Thu Nov 19 18:03:11 2015 >> > >> New Revision: 1715228 >> > >> >> > >> URL: http://svn.apache.org/viewvc?rev=1715228&view=rev >> > >> Log: >> > >> Add a tiny bit of code to allow testing with Apache Serf's http/2 >> > >> support. >> > >> >> > >> I committed this patch to celebrate that I got through basic_tests.py >> > >> using the current http/2 support. >> > >> >> > >> * subversion/libsvn_ra_serf/util.c >> > >> (conn_negotiate_protocol): New function. >> > >> (conn_setup): Register usage of conn_negotiate_protocol when >> > >> a very recent (read: trunk) serf + SVN__SERF_TEST_HTTP2 are used. >> > > >> > > Currently most tests pass for me over http2 when running with some >> minor >> > tweaks. I still get some aborted connections that aren't retried as >> > cleanly as >> > with http 1.1. Not sure what causes these yet; but this could also be an >> httpd >> > issues. >> > > >> > > The only ra function that really causes problems is the implementation of >> > the replay >> > > range api. This 100% expects that results will be received in the same >> order >> > in which >> > > they are sent. This is typically the correct way in http 1.1 using serf, >> > > but >> > even there it >> > > is sometimes possible to get results out of order. (Authentication can >> > > re- >> > queue >> > > requests... and sometimes authorization is necessary even when earlier >> > requests >> > > succeeded, depending on the auth scheme). >> > > >> > Can we use WINDOW_UPDATE frame to control flow of replay-revision >> > response? In this case we can use spillbuf for buffering responses to >> > avoid latency performance regressions. What do you think? >> >> In theory we could... we could set the initial window to 0 and then send a >> window update later. Assuming that httpd does that efficiently, etc. etc. >> (Some of these assumptions are most likely not true) >> >> But I doubt if it would be more efficient than just sending the request when >> we need it... or perhaps asking the properties a revision earlier. (We can >> also >> send the request, but just leave out the end of stream marker... Sending EOS >> will then handle the request) > > The proper 2.0 way would be to make the streams completely dependent on each > other via priority handling. > > But using this priority information is optional on the server side. And I > doubt that > we can trust this to always do the right thing even if httpd would implement > this > correctly today. > Yes, stream priorities is another option, but it's not reliable. We still have to manage spillbuf as fallback etc.
I agree that current code could fail in some edge cases, like re-authentication for some reason. I think simple solution for all these kind of problems would be just add new replay-range REPORT that will stream all revision range in one REPORT. We already do this for blame in file-revs REPORT, so I don't see problems here. For older servers we can just disable pipelining in this case. What do you think? -- Ivan Zhakov
