[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Merging parallel-put to /trunk

From: Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
Date: Sat, 30 Jan 2016 00:35:12 +0300

Stefan Fuhrmann <stefanfuhrmann_at_alice-dsl.de> writes:

> This branch adds support for HTTP2-style transactions to all backends.
> The feature is opt-in and supported in all 3 FS implementations.
> I'd like to merge this to /trunk next weekend, so that e.g. Bert can
> continue the work on the network layer. Because it is opt-in, removing
> the feature later should be simple - in case we find a major problem
> with it.
> Any objections?

I have several concerns about the particular design and implementation:

(1) Requiring twice more disk writes and space

  Changes that don't fit into memory require twice more disk writes and space
  in the parallel mode, because data is first spilled to a temporary file, and
  then appended to the protorev file. Even if we negotiate the use of this
  feature, and only enable it for HTTP/2 capable clients and servers, that
  might still be slower for every case except when committing over a
  high-latency network.

(2) Making the feature opt-in

  Since the feature is opt-in and disabled by default, we would be adding
  another layer to the amount of different code execution paths that we need
  to support and test (FS formats, addressing, parallel / non-parallel writes).
  And we cannot enable it by default due to (1).

(3) Opening a denial of service attack vector

  In the parallel mode, a server holds up to 0.5 MB of data in memory per
  every change. Combined with the fact that writes to the protorev files
  are serialized via rev-lock, an evil doer could be able to exhaust all
  available memory on the server by issuing a huge PUT request, waiting for
  the moment when the rev-lock is acquired and spamming with a big amount of
  smaller PUT requests. The latter requests would block until the rev-lock
  file is released, but the associated data is going to be kept in memory.
  Alternatively, this could lead to a starvation between httpd workers, where
  most of them are blocked waiting for a single PUT to complete and are
  locking out other clients.

(4) Being incompatible with released versions of Subversion

  Support for parallel writes is implemented without a format bump, and can
  result in failing commits or data corruption due to mismatching concurrency
  settings. Consider an environment with a reverse proxy doing load balancing
  between 1.9 and 1.10 mod_dav_svn servers that work with the same repository.
  A client performs two parallel PUT requests, and each of them is rerouted to
  each of the servers. Consequently, either the 1.9 server fails to acquire
  a nonblocking rev-lock and the commit fails, or, in a racy scenario, the
  file/offset manipulation logic in transaction.c:rep_write_contents_close()
  results in a corrupted revision.

Evgeny Kotkov
Received on 2016-01-29 22:35:42 CET

This is an archived mail posted to the Subversion Dev mailing list.