Stefan Sperling wrote on Sat, 28 Mar 2020 11:32 +0100:
> On Fri, Mar 27, 2020 at 10:58:48PM +0000, Daniel Shahaf wrote:
> > Stefan Sperling wrote on Fri, 27 Mar 2020 14:41 +0100:
> > > On Thu, Mar 26, 2020 at 08:32:38PM +0000, Daniel Shahaf wrote:
> > > > What I'm worried about is that we'll apply the "backwards compatibility
> > > > until 2.0" promise to something we shouldn't. As I said, issues of
> > > > this nature can't generally be found by tests; they can only be found
> > > > by code review.
> > >
> > > I'm not worried about that. After reading through the patch I don't see
> > > any reason why we would not want to commit to supporting this feature.
> > >
> >
> > I wasn't discussing the build-repcache patch specifically; I was
> > commenting in general terms about the risks of rushing patch review
> > processes lest the patches in question miss a minor release train.
> > "We'll just fix it in a patch release" is the right attitude, but
> > doesn't always apply.
>
> Ah, I see. That wasn't clear to me.
Well, I'm glad there is still one mailing list where people can sort out
disagreements without resorting to pyromania. :-)
> > Well, I guess the documentation, once written, should point out that if
> > any single revision build-repcache operates on adds a large number of
> > reps, commits made in parallel to the build-repcache run will be starved
> > out of adding their own rep-cache entries, which will manifest as
> > "post-commit FS processing failed". (We have a bug filed for this, but
> > I can't find its number right now, sorry.)
>
> Good point.
>
> Maybe the command's help output should mention that the repository will
> be blocked for commits while this command is running? Even if that isn't
> entirely true at the implementation level it would be good user-facing
> advice.
I don't think inaccuracies in the documentation are a good idea. The
proposal might discourage people from running this when they could;
might lead people to run this in order to trigger the side effect of
blocking commits for a short while, only to find commits did happen
successfully; and might get people to distrust our documentation in
general.
Therefore, I'd document the semantics accurately. If need be, we can
refer to fuller documentation in the release notes and/or the book.
> > > I know of at least two cases where rep-sharing was temporarily or perhaps
> > > even permanently disabled after SHA1 collisions were committed (one case
> > > is webkit, I learned about another case via elego's SVN support).
> > > These repositories are now growing faster than they would if rep-caching
> > > could be re-enabled easily.
> >
> > I don't follow. There's nothing stopping the admins of those
> > repositories from reënabling rep-cache as soon as they upgrade to
> > a Subversion release that has the sha1 collision fixes. That will solve
> > the growth problem going forward. Compared to this, the benefits of
> > running build-repcache on those repositories are limited.
>
> Ah, yes. You're right about that.
>
> I suspect some might have left the rep-cache disabled anyway cause
> of (unwarranted) trust issues.
>
> Apart from the above, there's also a small possibility that a rep-cache DB
> becomes corrupt or somehow unusable by sqlite. In such cases this command
> would be very useful.
+1
Come to think of it, mightn't it also be useful in plain «svnadmin load»
use-cases? Disable rep-cache, run «svnadmin load», then after it's done
go back and populate the rep-cache. Or even run «svnadmin load
--use-post-commit-hook» with a post-commit hook that forks off an
«svnadmin build-rep -r $REV» process, to have rep-cache population happen
in a separate thread.
Received on 2020-03-28 22:54:44 CET