On 2 September 2015 at 17:01, Johan Corveleyn <jcorvel_at_gmail.com> wrote:
> On Wed, Sep 2, 2015 at 2:43 PM, Evgeny Kotkov
> <evgeny.kotkov_at_visualsvn.com> wrote:
>> Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com> writes:
>>
>>>> Given the above, I am -1 on this optimization for svnadmin load-revprops
>>>> that was implemented in r1698359 and r1700305. Please revert these
>>>> changes.
>>>
>>> Thinking about alternative solutions I found that simply having a buffering
>>> wrapper without mark support would still eliminate the OS overhead and parts
>>> of the internal overhead. That would address all the points you have made
>>> above while still providing a decent improvement.
>>>
>>> IOW, revert r1700305 and rework / reduce / simplify the code changed
>>> by r1698359.
>>
>> I stated my veto and provided the justification that covers both r1700305
>> and r1698359. So, could you please revert both of them? Reworking and
>> adding changes on top of it is going to increase the mess and will be harder
>> to review.
>
> ISTR that in this community we try to treat a veto as a signal that
> further discussion is needed, or that more work is needed to resolve
> the question / issue. You may ask / suggest a revert if you think
> that's best, but it's mainly up for discussion how to best resolve
> things (and other avenues, such as further commits, may also resolve
> the issue).
>
You're right, but the problem is that the discussion doesn't happen.
Instead of this new code is committed (usually broken again) [1].
Let summarize what happened from my point of view:
1. An optimization was implemented in r1698359
2. Evgeny raised his concerns about possible unbounded memory usage
3. Some fix was applied in r1700305 (no discussion happened)
4. Evgeny vetoed both of them.
5. The original optimization was reworked (no discussion happened)
6. Another problem was found in the new code [1]
7. ???
This reminds me of the story we had with L1 DAG node cache -- see
"Unbounded memory usage and DoS possibility in mod_dav_svn / FSFS DAG
node cache" in private_at_s.a.o.
[1] http://svn.haxx.se/dev/archive-2015-09/0010.shtml
--
Ivan Zhakov
Received on 2015-09-02 17:40:16 CEST