Few comments. Partition & State should be in model package instead of config. Not sure where should we have Scope class.
HelixProperty should not contain batchMessage mode, bucket size etc. This existed before and it is having too many things. The main purpose of this class is to represent a HelixRecord. Here is the high level overview of how data is organized in zookeeper and how it is represented in the code. Every Zookeeper path that contains a set of child nodes is a table. We every ZNode represents a HelixProperty. Each HelixProperty consists of two things * Key: this is can be single key or composite/heirarchical. We use PropertyKey to represent this. * Value: This represents the content of the znode. We use ZNRecord to represent this. Most classes like IdealState, ExternalView etc are of HelixProperty type. The reason for doing this is, we have only one class (DataAccessor) to interact with the zookeeper layer. This DataAccessor class has to support simple CRUD operations. Does this make sense? On Sat, Mar 1, 2014 at 5:09 PM, kishore g <[email protected]> wrote: > Awesome work, I am review the api package. Can you provide some additional > info on commons-discovery ?. Is this short term or long term, whats the > value add. > > Initial code structure looks good package looks good. Agree with I* > classes. We should bring all model classes here. > > I did see some formatting issues. I think Kanak has a eclipse style xml > that fixes the formatting issues. Not a big issue. > > To setup eclipse, we need to first run mvn install -DskipTests. > > What are the next steps. If I understand correctly we will move model and > listener api's. I think we should change the listener interfaces and create > event classes. Also we have overloaded INIT and FINALIZE callback that has > caused some issues. > > Do we have a JIRA, we can add things we want to fix as subTasks and we > revisit later. > > thanks, > Kishore G > > > On Sat, Mar 1, 2014 at 4:53 PM, Sandeep Nayak <[email protected]> wrote: > >> Hey guys, >> >> I checked in some work into the branch >> https://github.com/kanakb/helix. Here is a summary >> >> (1) I have ZNRecord and ZNRecordDelta are kept as is in api. I think >> we need to revisit them and rework them >> >> (2) I have all ids retained with the json deserialization info. >> Removing that would have been a lot of rework and I was focussed on >> moving files into API this first go around. We need to clean up the >> @Json annotations >> >> (3) I have a model package but I decided to hold off on moving all >> models into that package because there were some internal >> dependencies, so I opted to use interfaces for the time being until we >> untangle some of the dependencies of model on core at which point we >> can drop the interfaces and move the model into api cleanly. So bear >> with those IInterface naming conventions, its an eyesore for a reason. >> >> (4) Please note the builder pattern I introduced, there are abstract >> builders in API and the implementations are in core. I decided to >> avoid writing plumbing and get something quick going so leveraged >> apache commons discovery. You can see the pattern in the builder >> packages in api. >> >> (5) API has third party dependencies on google-guava, >> apache-commons-discovery and jackson. We need to bring this down to a >> very minimum as we work through the API cleanup >> >> (6) I added a SerializableZNRecord in org.apache.helix in core because >> I did not want serializer references in the api package. It felt like >> an implementation detail >> >> (7) I added a AbstractRebalancerConfig in >> org.apache.helix.controller.rebalancer.config package in core. Reason >> was the serializer references again. >> >> (8) Most apis use <T extends interface> pattern so that we dont have >> to cast objects in the core project. >> >> (9) Note the ids classes are final deliberately. I would like to keep >> the constructors private but they are accessed in some places so had >> to keep them open. >> >> (10) I moved most of the enums into api. >> >> (11) StateModelDefId was referencing an internal class for a constant >> "SchedulerTaskQueue", I moved that into api and now everywhere the >> core class was referenced I reference the constant id value. >> >> I think I got everything, there is still a lot to be done but I did >> not want to keep going off the deep end and wanted to give everyone a >> chance to review, so we can rework as necessary. >> >> Apologies if I have missed out on formatting in some places, I tried >> to do a thorough job but I am sure I have fallen short in places. It >> has been quite some time since I worked on maven builds so let me know >> if I missed anything in the maven builds. >> >> Thanks, >> >> Sandeep >> > >
