You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2015/11/26 09:05:36 UTC

svn commit: r1716575 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Author: rhuijben
Date: Thu Nov 26 08:05:36 2015
New Revision: 1716575

URL: http://svn.apache.org/viewvc?rev=1716575&view=rev
Log:
In ra_serf: when a to-be committed file has text and property changes to be
applied, pipeline both requests.

* subversion/libsvn_ra_serf/commit.c
  (apply_textdelta): Add comment noting why the suggested EAGAIN approach
     doesn't work as simple as this.
  (close_file): Pipeline both requests if there are multiple requests. For
     simplicity just wait for the propfind to be done before waiting for
     the put. We have to wait for both anyway.

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__unschedule_handler): Add prototype.

* subversion/libsvn_ra_serf/util.c
  (svn_ra_serf__unschedule_handler): Make library private.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/commit.c
    subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
    subversion/trunk/subversion/libsvn_ra_serf/util.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=1716575&r1=1716574&r2=1716575&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Thu Nov 26 08:05:36 2015
@@ -1257,6 +1257,7 @@ open_root(void *edit_baton,
   const char *proppatch_target = NULL;
   apr_pool_t *scratch_pool = svn_pool_create(dir_pool);
 
+  SVN_DBG(("Root open: %d", base_revision));
   commit_ctx->open_batons++;
 
   if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(commit_ctx->session))
@@ -1446,6 +1447,8 @@ delete_entry(const char *path,
   const char *delete_target;
   svn_error_t *err;
 
+  SVN_DBG(("Rev: %d", (int)revision));
+
   if (USING_HTTPV2_COMMIT_SUPPORT(dir->commit_ctx))
     {
       delete_target = svn_path_url_add_component2(
@@ -1912,6 +1915,15 @@ apply_textdelta(void *file_baton,
    * writing to a temporary file (ugh). A special svn stream serf bucket
    * that returns EAGAIN until we receive the done call?  But, when
    * would we run through the serf context?  Grr.
+   *
+   * BH: If you wait to a specific event... why not use that event to
+   *     trigger the operation?
+   *     Having a request (body) bucket return EAGAIN until done stalls
+   *     the entire HTTP pipeline after writing the first part of the
+   *     request. It is not like we can interrupt some part of a request
+   *     and continue later. Or somebody else must use tempfiles and
+   *     always assume that clients work this bad... as it only knows
+   *     for sure after the request is completely available.
    */
 
   ctx->stream = svn_stream_lazyopen_create(delayed_commit_stream_open,
@@ -1975,6 +1987,7 @@ close_file(void *file_baton,
 {
   file_context_t *ctx = file_baton;
   svn_boolean_t put_empty_file = FALSE;
+  svn_ra_serf__handler_t *handler = NULL;
 
   ctx->result_checksum = text_checksum;
 
@@ -1987,9 +2000,6 @@ close_file(void *file_baton,
   /* If we had a stream of changes, push them to the server... */
   if (ctx->svndiff || put_empty_file)
     {
-      svn_ra_serf__handler_t *handler;
-      int expected_result;
-
       handler = svn_ra_serf__create_handler(ctx->commit_ctx->session,
                                             scratch_pool);
 
@@ -2015,20 +2025,10 @@ close_file(void *file_baton,
       handler->header_delegate = setup_put_headers;
       handler->header_delegate_baton = ctx;
 
-      SVN_ERR(svn_ra_serf__context_run_one(handler, scratch_pool));
-
-      if (ctx->added && ! ctx->copy_path)
-        expected_result = 201; /* Created */
-      else
-        expected_result = 204; /* Updated */
-
-      if (handler->sline.code != expected_result)
-        return svn_error_trace(svn_ra_serf__unexpected_status(handler));
+      /* And schedule the request. We'll check the result later */
+      svn_ra_serf__request_create(handler);
     }
 
-  if (ctx->svndiff)
-    SVN_ERR(svn_io_file_close(ctx->svndiff, scratch_pool));
-
   /* If we had any prop changes, push them via PROPPATCH. */
   if (apr_hash_count(ctx->prop_changes))
     {
@@ -2046,6 +2046,37 @@ close_file(void *file_baton,
                                  proppatch, scratch_pool));
     }
 
+  if (handler)
+    {
+      /* We sent a PUT... Let's check the result */
+      svn_error_t *err;
+      int expected_result;
+
+      err = svn_ra_serf__context_run_wait(&handler->done, handler->session,
+                                          scratch_pool);
+
+      if (handler->scheduled)
+        {
+          /* We reset the connection (breaking  pipelining, etc.), as
+          if we didn't the next data would still be handled by this handler,
+          which is done as far as our caller is concerned. */
+          svn_ra_serf__unschedule_handler(handler);
+        }
+
+      SVN_ERR(err);
+
+      if (ctx->added && !ctx->copy_path)
+          expected_result = 201; /* Created */
+      else
+          expected_result = 204; /* Updated */
+
+      if (handler->sline.code != expected_result)
+          return svn_error_trace(svn_ra_serf__unexpected_status(handler));
+    }
+
+  if (ctx->svndiff)
+      SVN_ERR(svn_io_file_close(ctx->svndiff, scratch_pool));
+
   ctx->commit_ctx->open_batons--;
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1716575&r1=1716574&r2=1716575&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Thu Nov 26 08:05:36 2015
@@ -1522,6 +1522,11 @@ svn_ra_serf__error_on_status(serf_status
 svn_error_t *
 svn_ra_serf__unexpected_status(svn_ra_serf__handler_t *handler);
 
+/* Make sure handler is no longer scheduled on its connection. Resetting
+   the connection if necessary */
+void
+svn_ra_serf__unschedule_handler(svn_ra_serf__handler_t *handler);
+
 
 /* ###? */
 svn_error_t *

Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1716575&r1=1716574&r2=1716575&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Thu Nov 26 08:05:36 2015
@@ -1001,7 +1001,7 @@ svn_ra_serf__context_run_wait(svn_boolea
    will cancel all outstanding requests and prepare the connection
    for re-use.
 */
-static void
+void
 svn_ra_serf__unschedule_handler(svn_ra_serf__handler_t *handler)
 {
   serf_connection_reset(handler->conn->conn);



RE: svn commit: r1716575 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: donderdag 26 november 2015 10:27
> To: Bert Huijben <be...@qqmail.nl>
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1716575 - in
> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
> 
> On 26 November 2015 at 12:11, Bert Huijben <be...@qqmail.nl> wrote:
> >> -----Original Message-----
> >> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> >> Sent: donderdag 26 november 2015 09:54
> >> To: Bert Huijben <be...@qqmail.nl>
> >> Cc: dev@subversion.apache.org
> >> Subject: Re: svn commit: r1716575 - in
> >> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
> >>
> >> On 26 November 2015 at 11:41, Bert Huijben <be...@qqmail.nl> wrote:
> >> >> -----Original Message-----
> >> >> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> >> >> Sent: donderdag 26 november 2015 09:19
> >> >> To: dev@subversion.apache.org; Bert Huijben <be...@qqmail.nl>
> >> >> Subject: Re: svn commit: r1716575 - in
> >> >> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
> >> >>
> >> >> On 26 November 2015 at 11:05, <rh...@apache.org> wrote:
> >> >> >
> >> >> > Author: rhuijben
> >> >> > Date: Thu Nov 26 08:05:36 2015
> >> >> > New Revision: 1716575
> >> >> >
> >> >> > URL: http://svn.apache.org/viewvc?rev=1716575&view=rev
> >> >> > Log:
> >> >> > In ra_serf: when a to-be committed file has text and property
> changes
> >> to
> >> >> be
> >> >> > applied, pipeline both requests.
> >> >> >
> >> >> This could fail over HTTP/2 if both pipelined requests will be handled
> >> >> by different worker threads: FSFS doesn't allow concurrent access to
> >> >> transactions.
> >> >
> >> > I'm surprised to learn that. I would have guessed concurrent access was
> >> > always part of the design of the fs layer, by the way we use it in
> mod_dav.
> >> >
> >> > I hope we can somehow lift that restriction, as improving commit
> >> performance
> >> > over mod_dav is high on a few wish lists.
> >> >
> >> First of all I'm not sure that concurrent *writing* could speed up
> >> commit: there is no fsync() calls when working with transactions. As
> >> far I remember svn:// also waits for every TXN operation to complete
> >> before sending next one, so svn:// and http:// performance should be
> >> the same when working over high-latency network.
> >
> > No, in svn:// the client sends out all the requests and only peeks the
> > connection to see if there is waiting data (an error) during commit. The
> client
> > only waits for the server after the entire commit is completed. Not after
> every txn operation.
> Yes, you are right. But such error handling will make connection
> unusable after error, is not it? And as far I remember ra_svn doesn't
> have code to reconnect if needed.

No, it will recover just fine here. No problem at all for many versions. If it didn't you would see a broken connection at every commit with an out of date.

The cases where the connection broke is where it sends an error where a strict result was expected. 
All cases I know of were fixed on trunk though. (I added tests to explicitly trigger errors in each ra call)

Some fixes were backported to 1.8 / 1.9.

I did this in preparation for backporting the ra-session reuse work.

	Bert


Re: svn commit: r1716575 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 26 November 2015 at 12:11, Bert Huijben <be...@qqmail.nl> wrote:
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> Sent: donderdag 26 november 2015 09:54
>> To: Bert Huijben <be...@qqmail.nl>
>> Cc: dev@subversion.apache.org
>> Subject: Re: svn commit: r1716575 - in
>> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
>>
>> On 26 November 2015 at 11:41, Bert Huijben <be...@qqmail.nl> wrote:
>> >> -----Original Message-----
>> >> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> >> Sent: donderdag 26 november 2015 09:19
>> >> To: dev@subversion.apache.org; Bert Huijben <be...@qqmail.nl>
>> >> Subject: Re: svn commit: r1716575 - in
>> >> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
>> >>
>> >> On 26 November 2015 at 11:05, <rh...@apache.org> wrote:
>> >> >
>> >> > Author: rhuijben
>> >> > Date: Thu Nov 26 08:05:36 2015
>> >> > New Revision: 1716575
>> >> >
>> >> > URL: http://svn.apache.org/viewvc?rev=1716575&view=rev
>> >> > Log:
>> >> > In ra_serf: when a to-be committed file has text and property changes
>> to
>> >> be
>> >> > applied, pipeline both requests.
>> >> >
>> >> This could fail over HTTP/2 if both pipelined requests will be handled
>> >> by different worker threads: FSFS doesn't allow concurrent access to
>> >> transactions.
>> >
>> > I'm surprised to learn that. I would have guessed concurrent access was
>> > always part of the design of the fs layer, by the way we use it in mod_dav.
>> >
>> > I hope we can somehow lift that restriction, as improving commit
>> performance
>> > over mod_dav is high on a few wish lists.
>> >
>> First of all I'm not sure that concurrent *writing* could speed up
>> commit: there is no fsync() calls when working with transactions. As
>> far I remember svn:// also waits for every TXN operation to complete
>> before sending next one, so svn:// and http:// performance should be
>> the same when working over high-latency network.
>
> No, in svn:// the client sends out all the requests and only peeks the
> connection to see if there is waiting data (an error) during commit. The client
> only waits for the server after the entire commit is completed. Not after every txn operation.
Yes, you are right. But such error handling will make connection
unusable after error, is not it? And as far I remember ra_svn doesn't
have code to reconnect if needed.

>
> This is why 'in general' you get not identical error reporting on commits when you use
> svn:// (or svn+ssh://, etc.). In many cases the only failure you see is the text from the
> server as the client doesn't even know which file/directory failed during the commit.
>
> With a bit of luck you receive errors a bit earlier than the final commit step. But with
> <= 1.9.0 this didn't work for svn+ssh:// on Windows at all. (We handled the error for
> not implemented as if no data was waiting)
>
Ouch. I think it's just an example that async/delayed error handling
is very tricky.


-- 
Ivan Zhakov

RE: svn commit: r1716575 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: donderdag 26 november 2015 09:54
> To: Bert Huijben <be...@qqmail.nl>
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1716575 - in
> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
> 
> On 26 November 2015 at 11:41, Bert Huijben <be...@qqmail.nl> wrote:
> >> -----Original Message-----
> >> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> >> Sent: donderdag 26 november 2015 09:19
> >> To: dev@subversion.apache.org; Bert Huijben <be...@qqmail.nl>
> >> Subject: Re: svn commit: r1716575 - in
> >> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
> >>
> >> On 26 November 2015 at 11:05, <rh...@apache.org> wrote:
> >> >
> >> > Author: rhuijben
> >> > Date: Thu Nov 26 08:05:36 2015
> >> > New Revision: 1716575
> >> >
> >> > URL: http://svn.apache.org/viewvc?rev=1716575&view=rev
> >> > Log:
> >> > In ra_serf: when a to-be committed file has text and property changes
> to
> >> be
> >> > applied, pipeline both requests.
> >> >
> >> This could fail over HTTP/2 if both pipelined requests will be handled
> >> by different worker threads: FSFS doesn't allow concurrent access to
> >> transactions.
> >
> > I'm surprised to learn that. I would have guessed concurrent access was
> > always part of the design of the fs layer, by the way we use it in mod_dav.
> >
> > I hope we can somehow lift that restriction, as improving commit
> performance
> > over mod_dav is high on a few wish lists.
> >
> First of all I'm not sure that concurrent *writing* could speed up
> commit: there is no fsync() calls when working with transactions. As
> far I remember svn:// also waits for every TXN operation to complete
> before sending next one, so svn:// and http:// performance should be
> the same when working over high-latency network.

No, in svn:// the client sends out all the requests and only peeks the connection to see if there is waiting data (an error) during commit. The client only waits for the server after the entire commit is completed. Not after every txn operation.

This is why 'in general' you get not identical error reporting on commits when you use svn:// (or svn+ssh://, etc.). In many cases the only failure you see is the text from the server as the client doesn't even know which file/directory failed during the commit.

With a bit of luck you receive errors a bit earlier than the final commit step. But with <= 1.9.0 this didn't work for svn+ssh:// on Windows at all. (We handled the error for not implemented as if no data was waiting)

	Bert


Re: svn commit: r1716575 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 26 November 2015 at 12:03, Bert Huijben <be...@qqmail.nl> wrote:
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> Sent: donderdag 26 november 2015 09:54
>> To: Bert Huijben <be...@qqmail.nl>
>> Cc: dev@subversion.apache.org
>> Subject: Re: svn commit: r1716575 - in
>> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
>>
>> On 26 November 2015 at 11:41, Bert Huijben <be...@qqmail.nl> wrote:
>> >> -----Original Message-----
>> >> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> >> Sent: donderdag 26 november 2015 09:19
>> >> To: dev@subversion.apache.org; Bert Huijben <be...@qqmail.nl>
>> >> Subject: Re: svn commit: r1716575 - in
>> >> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
>> >>
>> >> On 26 November 2015 at 11:05, <rh...@apache.org> wrote:
>> >> >
>> >> > Author: rhuijben
>> >> > Date: Thu Nov 26 08:05:36 2015
>> >> > New Revision: 1716575
>> >> >
>> >> > URL: http://svn.apache.org/viewvc?rev=1716575&view=rev
>> >> > Log:
>> >> > In ra_serf: when a to-be committed file has text and property changes
>> to
>> >> be
>> >> > applied, pipeline both requests.
>> >> >
>> >> This could fail over HTTP/2 if both pipelined requests will be handled
>> >> by different worker threads: FSFS doesn't allow concurrent access to
>> >> transactions.
>> >
>> > I'm surprised to learn that. I would have guessed concurrent access was
>> > always part of the design of the fs layer, by the way we use it in mod_dav.
>> >
>> > I hope we can somehow lift that restriction, as improving commit
>> performance
>> > over mod_dav is high on a few wish lists.
>> >
>> First of all I'm not sure that concurrent *writing* could speed up
>> commit: there is no fsync() calls when working with transactions. As
>> far I remember svn:// also waits for every TXN operation to complete
>> before sending next one, so svn:// and http:// performance should be
>> the same when working over high-latency network.
>>
>> Another problem could be than TXN operations are have dependencies on
>> each other and order is very important.
>
> That assumes that the limit is on the server side.
>
> I usually commit halfway over the earth... my limits are 100% in the pipeline and
> mostly in the latency between client and server. (100Mbit+ connections on both
> sides). Sending concurrent requests would be a huge performance boost.

Yes, it will be the boost in high-latency connection, but it could
degrade performance in low-latency network environment due high
concurrency.

>
>
> But if we require everything in a strict order we should just immediately stop
> using multiple requests. If serialized access is the only thing our backends
> support we should just send one huge commit request. That saves a huge number
> of synchronize events between client and server.
>
> Currently we send a request... wait for the result... (server waiting for us) send a request... wait for the result.
>
Just to clarify: concurrency limitation is only applies for write
operations to TXN. Removing this limitation will be very challenging
task IMO. It this could reduce server-side performance since we would
need more syncronization, cache invalidation etc.

One huge commit request has another problems:
1. Our commit editor API performs operations one by one and caller
expect to get operation result early, instead of final txn commit
stage.

2. In HTTP/1.1 there is no way to signal client that it should stop
sending data due error: server expected to read all request body (and
discard) to send response. But this was improved in HTTP/2 though.


> And more of that...
>
> Removing a tempfile here or there isn't going to help if we can't remove the waiting.
> We can't change the wire (or light) speed between client and server.
>
Yes, we cannot change the light speed as far I know :)

Our tests shows that commit operation is relatively even when done to
the local server and there are some room for improvements.

> We could design the request to return the same intermediate results in the request
> body as we do now, with only a fraction of the synchronization points.
>
> But this is the other way around as the intensions we had with HTTPv2 around 1.7.
> There we tried to do things the HTTP way by optimal pipelining.
>
I was really surprised that HTTP/2 dropped option to send strict
ordered HTTP request. I think it's serious protocol regression.

-- 
Ivan Zhakov

RE: svn commit: r1716575 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: donderdag 26 november 2015 09:54
> To: Bert Huijben <be...@qqmail.nl>
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1716575 - in
> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
> 
> On 26 November 2015 at 11:41, Bert Huijben <be...@qqmail.nl> wrote:
> >> -----Original Message-----
> >> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> >> Sent: donderdag 26 november 2015 09:19
> >> To: dev@subversion.apache.org; Bert Huijben <be...@qqmail.nl>
> >> Subject: Re: svn commit: r1716575 - in
> >> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
> >>
> >> On 26 November 2015 at 11:05, <rh...@apache.org> wrote:
> >> >
> >> > Author: rhuijben
> >> > Date: Thu Nov 26 08:05:36 2015
> >> > New Revision: 1716575
> >> >
> >> > URL: http://svn.apache.org/viewvc?rev=1716575&view=rev
> >> > Log:
> >> > In ra_serf: when a to-be committed file has text and property changes
> to
> >> be
> >> > applied, pipeline both requests.
> >> >
> >> This could fail over HTTP/2 if both pipelined requests will be handled
> >> by different worker threads: FSFS doesn't allow concurrent access to
> >> transactions.
> >
> > I'm surprised to learn that. I would have guessed concurrent access was
> > always part of the design of the fs layer, by the way we use it in mod_dav.
> >
> > I hope we can somehow lift that restriction, as improving commit
> performance
> > over mod_dav is high on a few wish lists.
> >
> First of all I'm not sure that concurrent *writing* could speed up
> commit: there is no fsync() calls when working with transactions. As
> far I remember svn:// also waits for every TXN operation to complete
> before sending next one, so svn:// and http:// performance should be
> the same when working over high-latency network.
> 
> Another problem could be than TXN operations are have dependencies on
> each other and order is very important.

That assumes that the limit is on the server side.

I usually commit halfway over the earth... my limits are 100% in the pipeline and mostly in the latency between client and server. (100Mbit+ connections on both sides). Sending concurrent requests would be a huge performance boost.


But if we require everything in a strict order we should just immediately stop using multiple requests. If serialized access is the only thing our backends support we should just send one huge commit request. That saves a huge number of synchronize events between client and server.

Currently we send a request... wait for the result... (server waiting for us) send a request... wait for the result.

And more of that...

Removing a tempfile here or there isn't going to help if we can't remove the waiting. We can't change the wire (or light) speed between client and server.

We could design the request to return the same intermediate results in the request body as we do now, with only a fraction of the synchronization points.



But this is the other way around as the intensions we had with HTTPv2 around 1.7. There we tried to do things the HTTP way by optimal pipelining.

	Bert


Re: svn commit: r1716575 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 26 November 2015 at 11:41, Bert Huijben <be...@qqmail.nl> wrote:
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> Sent: donderdag 26 november 2015 09:19
>> To: dev@subversion.apache.org; Bert Huijben <be...@qqmail.nl>
>> Subject: Re: svn commit: r1716575 - in
>> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
>>
>> On 26 November 2015 at 11:05, <rh...@apache.org> wrote:
>> >
>> > Author: rhuijben
>> > Date: Thu Nov 26 08:05:36 2015
>> > New Revision: 1716575
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1716575&view=rev
>> > Log:
>> > In ra_serf: when a to-be committed file has text and property changes to
>> be
>> > applied, pipeline both requests.
>> >
>> This could fail over HTTP/2 if both pipelined requests will be handled
>> by different worker threads: FSFS doesn't allow concurrent access to
>> transactions.
>
> I'm surprised to learn that. I would have guessed concurrent access was
> always part of the design of the fs layer, by the way we use it in mod_dav.
>
> I hope we can somehow lift that restriction, as improving commit performance
> over mod_dav is high on a few wish lists.
>
First of all I'm not sure that concurrent *writing* could speed up
commit: there is no fsync() calls when working with transactions. As
far I remember svn:// also waits for every TXN operation to complete
before sending next one, so svn:// and http:// performance should be
the same when working over high-latency network.

Another problem could be than TXN operations are have dependencies on
each other and order is very important.

-- 
Ivan Zhakov

RE: svn commit: r1716575 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: donderdag 26 november 2015 09:19
> To: dev@subversion.apache.org; Bert Huijben <be...@qqmail.nl>
> Subject: Re: svn commit: r1716575 - in
> /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c
> 
> On 26 November 2015 at 11:05, <rh...@apache.org> wrote:
> >
> > Author: rhuijben
> > Date: Thu Nov 26 08:05:36 2015
> > New Revision: 1716575
> >
> > URL: http://svn.apache.org/viewvc?rev=1716575&view=rev
> > Log:
> > In ra_serf: when a to-be committed file has text and property changes to
> be
> > applied, pipeline both requests.
> >
> This could fail over HTTP/2 if both pipelined requests will be handled
> by different worker threads: FSFS doesn't allow concurrent access to
> transactions.

I'm surprised to learn that. I would have guessed concurrent access was always part of the design of the fs layer, by the way we use it in mod_dav.

I hope we can somehow lift that restriction, as improving commit performance over mod_dav is high on a few wish lists.

	Bert


Re: svn commit: r1716575 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 26 November 2015 at 11:05, <rh...@apache.org> wrote:
>
> Author: rhuijben
> Date: Thu Nov 26 08:05:36 2015
> New Revision: 1716575
>
> URL: http://svn.apache.org/viewvc?rev=1716575&view=rev
> Log:
> In ra_serf: when a to-be committed file has text and property changes to be
> applied, pipeline both requests.
>
This could fail over HTTP/2 if both pipelined requests will be handled
by different worker threads: FSFS doesn't allow concurrent access to
transactions.

-- 
Ivan Zhakov