I'm also +1 to at least taking them out for the intra-cluster protocols. -- Aaron T. Myers Software Engineer, Cloudera
On Sun, Mar 18, 2012 at 7:49 PM, Eli Collins <e...@cloudera.com> wrote: > On Sat, Mar 17, 2012 at 3:31 PM, Todd Lipcon <t...@cloudera.com> wrote: > > Hi all, > > > > I've been working on some patches recently that required adding a new > > protocol and some RPC calls that will be used entirely internally to > > HDFS -- i.e the types and functions are never exposed to clients. The > > process to do this involved: > > 1) Add a new .proto file MyProtocol.proto with the types and the > > service definition > > 2) Add a new empty Java interface MyProtocolPB.java which adds the > > ProtocolInfo and KerberosInfo annotations > > 3) Add a new Java interface MyProtocol.java which duplicates the same > > methods I defined in the protobuf service > > 4) For each new type, create a new Java class which duplicates the > > fields, getters, and setters from the protobuf messages > > 5) Create a Client-Side Translator and Server-Side Translator class, > > each containing a wrapper method for each of the calls > > 6) Create a PBHelper class which contains two convert() methods for > > each of the new types > > > > Given that we have many protocols that we never intend to expose, I > > see little benefit to adding the indirection layer here. It only makes > > the task of modifying the protocols quite onerous and full of > > duplicate boilerplate code. > > I'd like to propose that, when adding protocols that are meant to be > > HDFS-private, we drop steps 3-6 and use the protobuf RPC engine > > directly. Doing this doesn't force our hand or limit our options in > > the future -- should we want to add an alternate mechanism one can > > always add the indirection layer down the road. > > > > Thoughts? > > > > Sounds good to me. > > I'm not sure what the indirection layer really buys us even for the > client <-> server protocols since just PB should be sufficient there. > But that's a separate discussion. > > > > -Todd > > -- > > Todd Lipcon > > Software Engineer, Cloudera >
