Hi Sandeep, Yes, and I definitely appreciate you starting this thread. It's something that Helix doesn't really have right now and it's absolutely necessary for intuitive APIs.
For reference, here are how some other projects tackle this (not necessarily Helix-related): Cassandra: https://github.com/apache/cassandra/tree/trunk/src/java/org/apache/cassandra/exceptions (broken into themes like ConfigException, and then subclasses with the type -- similar to what you're suggesting)Hive: https://github.com/apache/hive (one exception per package, roughly)ZooKeeper: https://github.com/apache/zookeeper/blob/trunk/src/java/main/org/apache/zookeeper/KeeperException.java?source=c (all in one wrapper class, flat structure describing on the type of failure)Yarn: https://github.com/apache/hadoop-common/tree/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/exceptions (flat, class only describes the error type) Personally, I think a good scheme is to have a hierarchy, but with maybe only two levels: (1) the scope of the error, e.g. ConfigException, and (2) the cause, e.g. ConfigExistsException. Kanak ---------------------------------------- > Date: Wed, 19 Feb 2014 22:24:12 -0800 > Subject: Re: Exceptions in Helix > From: [email protected] > To: [email protected] > > Hi Kanak, > > Like I said the first stab was to start a conversation. So what you are > pointing at is defining an exception based on the API invoked. So a > NodeAlreadyExistsException can be thrown on an addNode API but that > exception should specialization of the ConfigException to clearly indicate > that the set of exceptions happen during configuration of the nodes/cluster. > > Sandeep > > > On Wed, Feb 19, 2014 at 10:08 PM, Kanak Biscuitwala > <[email protected]>wrote: > > > My concern with this hierarchy is that it does a good job of describing > > what fails, but doesn't really hint at why. For instance, when trying to > > create a cluster that already exists, we would get something like a > > ClusterConfigException, and need to read javadocs to understand how it > > could be thrown. An alternative is to create exceptions based on error > > type, like ExistsException or something. > > > > Anyone else have thoughts? > > > > > Date: Wed, 19 Feb 2014 21:48:00 -0800 > > > Subject: Exceptions in Helix > > > From: [email protected] > > > To: [email protected] > > > > > > Hi guys, > > > > > > I sent out a mail last night (on the users maillist) that currently > > > Instance configuration retrieval from HelixAdmin throws HelixException > > > which is a runtime exception. > > > > > > I did not want to create a whole exception hierarchy for every API given > > my > > > narrow scope of understanding but wanted to illustrate an exception > > > hierarchy focussed on the above aspect and maybe influence an exception > > > hierarchy design which I am hoping someone who has a better grasp on the > > > entire codebase can pick and flesh out further. > > > > > > I looked at > > https://cwiki.apache.org/confluence/display/HELIX/API+Redesign and > > > saw the configuration hierarchy. > > > > > > So given that context here is my first stab at the Helix configuration > > > exception hierarchy. > > > > > > > > > - *HelixException //**This is base for all checked exceptions in > > Helix* > > > - *HelixConfigException* extends HelixException //This represents > > > checked exception hierarchy for configuration exceptions > > > - *HelixResourceConfigException* extends HelixConfigException > > > - *HelixStateModelConfigException* extends HelixConfigException > > > - *HelixInstanceConfigException* extends HelixConfigException > > > //Config exception specific to instances being added > > > - *HelixSpectatorConfigException* extends > > > HelixNodeConfigException //Specialized exceptions for types > > *if > > > applicable or necessary* > > > - *HelixParticipantConfigException* extends > > > HelixNodeConfigException //Specialized exceptions for types > > *if > > > applicable or necessary* > > > - *HelixControllerConfigException* extends > > > HelixNodeConfigException //Specialized exceptions for types > > *if > > > applicable or necessary* > > > - Other Checked Exceptions which are non-config but identify > > other > > > aspects of Helix which represent distinct subsets of > > functionality, like > > > the above represents configuration part. > > > - *HelixRuntimeException //**Base for all exceptions from which Helix > > > cannot recover* > > > - All derived classes which are failures which cannot be recovered > > > from > > > > > > Again I am throwing this out to start a conversation and maybe influence > > a > > > design approach for exceptions in future releases. I apologize for not > > > spending time on drawing a neat diagram, I decided to something out > > sooner > > > rather than wait until I got together a pretty diagram. > > > > > > Hopefully, you guys get an idea of what I am trying to convey. Feel free > > to > > > send questions if you want clarity on my suggestion, but please recognize > > > that its only been less than a week that I have looked at Helix so I hope > > > you won't expect me to chalk up the entire hierarchy :-) I think one of > > you > > > guys who has worked on the codebase for sometime may be better suited. I > > > can certainly help review and/or fine-tune and in the process learn Helix > > > internals a bit more. > > > > > > Thanks, > > > > > > Sandeep > > > >
