Hi Vishal, thanks for the list. As you can see when we do find issues we do our best to address them and increase testing in that area. Unfortunately our testing regime, while extensive is not exhaustive. You can see the clover coverage reports here btw: https://hudson.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/clover/
We'd love to see further contributions around testing. Thomas has opened some discussion around code refactoring, and I'm hopeful that will increase the coverage and enable "design for test" which we lack in some cases. Patrick On Fri, Oct 15, 2010 at 12:24 PM, Vishal K <vishalm...@gmail.com> wrote: > 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 > > > >> > > > > > > > > > > > > >
