Thanks, Yann;
   This does help explain the rationale and I appreciate you taking the
time to walk us through the reasoning.

-- 
Daniel Ruggeri

On 5/20/2018 3:59 PM, Yann Ylavic wrote:
> On Tue, May 8, 2018 at 7:19 PM, Jim Jagielski <j...@jagunet.com> wrote:
>> I am under the impression is that we should likely restore mod_slotmem_shm
>> back to its "orig" condition,
> So I did this (r1831868),
>
>> either:
>>
>>    o
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/slotmem/mod_slotmem_shm.c?view=markup&pathrev=1822341
> and this too (r1831869).
>
>> Just for the heck of it, didn't r1822341 actually *FIX* the PR?
> Not enough to pass all the tests raised by both PRs 62044 and 62308,
> hence follow ups r1831870, r1831871+r1831935 and finally
> r1831938.
>
> I think I owe an explanation on to why I made those changes in
> mod_slotmem_shm for 2.4.33, and why until now it looked to me like the
> right thing to do.
>
> Initially I thought that, on unixes MPMs, mod_slotmem_shm was
> reusing/preserving SHMs on restart (some parts of the code were quite
> misleading in this regard), and thus switching to per generation SHMs
> (like on Windows) was potentially going to break Unix users.
> Actually we never have reused SHMs on restart on any system unless
> BalancerPersist is enabled, while for me BalancerPersist was meant to
> preserve data on stop/start only.
> In any case this led me to take the "SHMs maintained in pglobal"
> approach for all OSes (creating new ones only when sizes change), as I
> thought it would allow to preserve SHMs on Windows too (IOW, was meant
> to fix Windows rather than break Unix).
> This has been my reasoning from the start, I only cared about fixing
> the reported bugs and preserving the existing behaviour (supposedly),
> not really an irrepressible desire from me to change/refactor the code
> (as you have suggested several times).
>
> Anyway, this was before I started to work on the last issue reported
> in PR 62308 (change some BalancerMember name/port and httpd won't
> restart), where I realized that sharing SHMs between old and new
> generations can't work in all cases (at least without more non-small
> changes), even when the sizes don't change. So I wondered if this
> particular case worked in 2.4.29, and found that SHMs were re-created
> each time, so it worked.. until BalancerPersist was enabled (same
> failure).
>
> So you were right about the potential to break things with my changes
> in the code, though it didn't work as expected already.
> Btw, I would have preferred more constructive feedbacks, including in
> the discussions about PR 62044 and the original r1822341 commit thread
> (where I explained why I was going to revert it..).
>
> Now we are back to 2.4.29 code, r1822341 is in again, and I committed
> additional changes (minimal hopefully) to address the issues reported
> in PR 62308 (and PR 62044 still). My own testing, based on the tests
> run by the OP (his on Windows, mine on Linux), all passes right now.
> So I'm waiting for the OP's latest results to propose a backport.
>
> WDYT of this approach (and patches), do it sound better?
>
> Regards,
> Yann.

Reply via email to