cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Robin Green" <gree...@hotmail.com>
Subject Re: woah there - "vetoed" changes committed to Engine
Date Sat, 16 Sep 2000 23:04:38 GMT
Donald Ball <balld@webslingerZ.com> wrote:
>robin, i'm psyched to see another committer dedicated to keeping the
>cocoon1 branch up and running until cocoon2 comes around, but i got a
>little issue here. i voted -1 on the engine synch. patch since i thought
>that we shouldn't put in a patch that messes around with cocoon that
>deeply shortly before a new major release. according to the terms of the
>asf project constitution:
>
>http://java.apache.org/main/constitution.html
>
>my veto is binding unless you convince me otherwise.

Right. Sorry, I forgot about that. It's been a while since I read the 
constitution.

>i'm really not trying
>to be a tight-ass, i'm perfectly happy to switch my vote, but we oughta
>follow the rules - especially now that there are now, what, six, seven
>whole committers on the cocoon project? :)

Absolutely. I'm not the type of person who tends to break rules on a 
personal whim (by mistake, maybe; for a good reason, maybe; but not on a 
whim...).

>(note i'm speaking from experience here, having introduced a fairly
>serious synchronization bug shortly before a major release of cocoon. was
>it 1.3?)

Okay, here's my attempt at swaying you (and anyone else who has similar 
concerns). There are basically three changes here.

1. Fixing a bug that only allowed one request through the engine at a time 
(that's right - one thread per engine!), which was not what was intended at 
all, according to the submitter (i.e. I removed the synchronized (this) {} 
). I can't see how this change can be dangerous. It's just changing it back 
to how it was in 1.7.4. I think the original synrchronized block was just 
left in mistakenly.

2. Fixing a possible missed notify() by synchronizing fully on the Block 
object, rather than only part of the time. I can't see how this change could 
be dangerous either - indeed it prevents a bug that could have been very 
hard to find and could have caused mysterious timeouts on many sites on 
heavy load - when one thread unblocks before another thread is ready to 
receive the notification. That's a synchronization ABC thing, anyway - at my 
college it was exactly this concept of missed notify that was used to 
explain why synchronization is necessary.

3. A very primitive profiler that just takes the current time at various 
points and puts the results in a hashtable, that is disabled by default in 
cocoon.properties - so it can't do any damage. (Touch wood!)

(Why #3? Well, I had a pile up of things I wanted to make into patches when 
I submitted it as a patch, so I hurriedly put two of them together for 
convenience. With hindsight this was a bad idea.)

It looks to me like there are three options:

A. Undo these set of changes (or at least the synchronization changes), and 
end up with a release that is very likely more buggy. In my opinion 
certainly more buggy.

B. Keep them, and release on schedule, with only a few days testing period. 
We have Apache JMeter - that's good for testing multi-threading. (note that 
JMeter can also expose JVM bugs, not just Cocoon bugs.)

C. Keep them, but defer the release (but keep the feature-freeze) to allow a 
longer testing period. I'm not sure what length of testing period would be 
most suitable for this type of change.

In my opinion, C is most preferable, because it is most likely to work best 
at the end of it, followed by B, followed by A (least preferable).

Ultimately I feel that leaving these bugs in (with admittedly 
hard-to-quantify risks), out of a belief that the risks are higher if they 
are fixed, is a misjudgement. If you're going to leave bugs in software, 
there'd better be a good reason for it, which _outweighs_ the reason for 
fixing it - and I don't think it does in this case. (Obvious point, I know, 
but how else could I say it?)

These are really _simple_ fixes, measured against the standards of the 
software industry (multiple megabyte MS patches anyone? Okay maybe there's a 
little redundancy there, but still). Sure, any fix can introduce new bugs - 
but you could say that about anything, even correcting a comment (hey, you 
could delete the */ by accident :).

"Why do I think this is an issue worth arguing about?" you may be wondering. 
Well, it's simple - I'm embarassed with these particular bugs, and even more 
embarassed with a few other things like how-it-works.xml (which didn't even 
build, which hadn't been finished and which contained numerous mistakes 
which would be rather unhelpful to a newbie; so I've rewritten it. It was 
not written by any of the Cocoon committers, I hasten to add, just an 
enthusiastic outsider who thought he/she understood things better than 
he/she really did - it can happen to anyone!)

So, for the sake of the "dignity" of the Cocoon project overall, including 
myself, I'd like to get it in a more shipshape condition. Now you may say 
that's about image and PR and marketing etc. which we are things we should 
stop getting hung up about - but it's not just image, it's about code 
quality (and quality of the docs). And yes I admit it, I am being fussy, but 
we're all allowed our little weaknesses aren't we? :)

While I can see the sense in being cautious just before a release, as a 
general principle - if I were a complete outsider I expect I'd _still_ think 
that leaving these known simple bugs in was... odd, for an open source 
project.

--
Robin

P.S. Sorry - if I am being way too long-winded let me know. :(


_________________________________________________________________________
Get Your Private, Free E-mail from MSN Hotmail at http://www.hotmail.com.

Share information about yourself, create your own public profile at 
http://profiles.msn.com.


Mime
View raw message