Hi Patrick, On Fri, Oct 15, 2010 at 2:22 PM, Patrick Hunt <ph...@apache.org> wrote:
> > Recently, we have ran into issues in ZK that I believe should have caught > by some basic testing before the release > > Vishal, can you be more specific, point out specific JIRAs that you entered > would be very valuable. Don't worry about hurting our feelings or anything, > without this type of feedback we can't address the specific issues and > their > underlying problems. > > Heres a list of few issues: Leader election taking a long time to complete - https://issues.apache.org/jira/browse/ZOOKEEPER-822 Last processed zxid set prematurely while establishing leadership - https://issues.apache.org/jira/browse/ZOOKEEPER-790 FLE implementation should be improved to use non-blocking sockets ZOOKEEPER-900 ZK lets any node to become an observer - https://issues.apache.org/jira/browse/ZOOKEEPER-851 > Regards, > > Patrick > > On Fri, Oct 15, 2010 at 11:14 AM, Mahadev Konar <maha...@yahoo-inc.com > >wrote: > > > Well said Vishal. > > > > I really like the points you put forth!!! > > > > Agree on all the points, but again, all the point you mention require > > commitment from folks like you. Its a pretty hard task to test all the > > corner cases of ZooKeeper. I'd expect everyone to pitch in for testing a > > release. We should definitely work towards a plan. You should go ahead > and > > create a jira for the QA plan. We should all pitch in with what all > should > > be tested. > > > > Thanks > > mahadev > > > > On 10/15/10 7:32 AM, "Vishal K" <vishalm...@gmail.com> wrote: > > > > > Hi, > > > > > > I would like to add my few cents here. > > > > > > I would suggest to stay away from code cleanup unless it is absolutely > > > necessary. > > > > > > I would also like to extend this discussion to understand the amount of > > > testing/QA to be performed before a release. How do we currently > qualify > > a > > > release? > > > > > > Recently, we have ran into issues in ZK that I believe should have > caught > > by > > > some basic testing before the release. I will be honest in saying that, > > > unfortunately, these bugs have resulted in questions being raised by > > several > > > people in our organization about our choice of using ZooKeeper. > > > Nevertheless, our product group really thinks that ZK is a cool > > technology, > > > but we need to focus on making it robust before adding major new > features > > to > > > it. > > > > > > I would suggest to: > > > 1. Look at current bugs and see why existing test did not uncover these > > bugs > > > and improve those tests. > > > 2. Look at places that need more tests and broadcast it to the > community. > > > Follow-up with test development. > > > 3. Have a crisp release QA strategy for each release. > > > 4. Improve API documentation as well as code documentation so that the > > API > > > usage is clear and debugging is made easier. > > > > > > Comments? > > > > > > Thanks. > > > -Vishal > > > > > > On Fri, Oct 15, 2010 at 9:44 AM, Thomas Koch <tho...@koch.ro> wrote: > > > > > >> Hi Benjamin, > > >> > > >> thank you for your response. Please find some comments inline. > > >> > > >> Benjamin Reed: > > >>> code quality is important, and there are things we should keep in > > >>> mind, but in general i really don't like the idea of risking code > > >>> breakage because of a gratuitous code cleanup. we should be watching > > out > > >>> for these things when patches get submitted or when new things go in. > > >> I didn't want to say it that clear, but especially the new Netty code, > > both > > >> on > > >> client and server side is IMHO an example of new code in very bad > shape. > > >> The > > >> client code patch even changes the FindBugs configuration to exclude > the > > >> new > > >> code from the FindBugs checks. > > >> > > >>> i think this is inline with what pat was saying. just to expand a > bit. > > >>> in my opinion clean up refactorings have the following problems: > > >>> > > >>> 1) you risk breaking things in production for a potential future > > >>> maintenance advantage. > > >> If your code is already in such a bad shape, that every change > includes > > >> considerable risk to break something, then you already are in trouble. > > With > > >> every new feature (or bugfix!) you also risk to break something. > > >> If you don't have the attitude of permanent refactoring to improve the > > code > > >> quality, you will inevitably lower the maintainability of your code > with > > >> every > > >> new feature. New features will build on the dirty concepts already in > > the > > >> code > > >> and therfor make it more expensive to ever clean things up. > > >> > > >>> 2) there is always subjectivity: quality code for one code quality > > >>> zealot is often seen as a bad design by another code quality zealot. > > >>> unless there is an objective reason to do it, don't. > > >> I don't agree. IMHO, the area of subjectivism in code quality is > > actually > > >> very > > >> small compared to hard established standards of quality metrics and > best > > >> practices. I believe that my list given in the first mail of this > thread > > >> gives > > >> examples of rather objective guidelines. > > >> > > >>> 3) you may cleanup the wrong way. you may restructure to make the > > >>> current code clean and then end up rewriting and refactoring again to > > >>> change the logic. > > >> Yes. Refactoring isn't easy, but necessary. Only over time you better > > >> understand your domain and find better structures. Over time you > > introduce > > >> features that let code grow so that it should better be split up in > > smaller > > >> units that the human brain can still handle. > > >> > > >>> i think we can mitigate 1) by only doing it when necessary. as a > > >>> corollary we can mitigate 2) and 3) by only doing > refactoring/cleanups > > >>> when motivated by some new change: fix a bug, increased performance, > > new > > >>> feature, etc. > > >> I agree that refactoring should be carefully planned and done in small > > >> steps. > > >> Therefor I collected each refactoring item for the java client in a > > small > > >> separate bug in > https://issues.apache.org/jira/browse/ZOOKEEPER-835that > > >> can > > >> individually be discussed, reviewed and tested. > > >> > > >> Have a nice weekend after Hadoop World! > > >> > > >> Thomas Koch, http://www.koch.ro > > >> > > > > > > > >