Hey Gwen, This makes sense to me.
A couple of thoughts, mostly confirming what you said I think: 1. Ideally we would move completely over to the new style of request definition for server-side processing, even for the internal requests. This way all requests would have the same header/body struct stuff. As you say for the internal requests we can just delete the scala code. For the old clients they will continue to use their old request definitions until we eol them. I would propose that new changes will go only into the new request/response objects and the old scala ones will be permanently stuck on their current version until discontinued. So after this change that old scala code could be considered frozen. 2. I think it would be reasonable to keep all the requests under common, even though as you point out there is currently no use for some of them beyond broker-to-broker communication at the moment. 3. We should think a little about how versioning will work. Making this convenient on the server side is an important goal for the new style of request definition. At the serialization level we now handle versioning but the question we should discuss and work out is how this will map to the request objects (which I assume will remain unversioned). 4. Ideally after this refactoring the network package should not be dependent on the individual request objects. The intention is that stuff in kafka.network is meant to be generic network infrastructure that doesn't know about the particular fetch/produce apis we have implemented on top. -Jay On Wed, Mar 18, 2015 at 11:11 AM, Gwen Shapira <gshap...@cloudera.com> wrote: > Hi Jun, > > I was taking a slightly different approach. Let me know if it makes > sense to you: > > 1. Get the bytes from network (kinda unavoidable...) > 2. Modify RequestChannel.Request to contain header and body (instead > of a single object) > 3. Create the head and body from bytes as follow: > val header: RequestHeader = RequestHeader.parse(buffer) > val apiKey: Int = header.apiKey > val body: Struct = > ProtoUtils.currentRequestSchema(apiKey).read(buffer).asInstanceOf[Struct] > 4. KafkaAPIs will continue getting RequestChannel.Request, but will > now have access to body and header separately. > > I agree that I need a Request/Response objects that contain only the > body for all requests objects. > I'm thinking of implementing them in o.a.k.Common.Requests in Java for > consistency. > > When we are discussing the requests/responses used in SimpleConsumer, > we mean everything used in javaapi, right? > > Gwen > > > > On Wed, Mar 18, 2015 at 9:55 AM, Jun Rao <j...@confluent.io> wrote: > > Hi, Gwen, > > > > I was thinking that we will be doing the following in KAFKA-1927. > > > > 1. Get the bytes from network. > > 2. Use a new generic approach to convert bytes into request objects. > > 2.1 Read the fixed request header (using the util in client). > > 2.2 Based on the request id in the header, deserialize the rest of the > > bytes into a request specific object (using the new java objects). > > 3. We will then be passing a header and an AbstractRequestResponse to > > KafkaApis. > > > > In order to do that, we will need to create similar request/response > > objects for internal requests such as StopReplica, LeaderAndIsr, > > UpdateMetadata, ControlledShutdown. Not sure whether they should be > written > > in java or scala, but perhaps they should be only in the core project. > > > > Also note, there are some scala requests/responses used directly in > > SimpleConsumer. Since that's our public api, we can't remove those scala > > objects until the old consumer is phased out. We can remove the rest of > the > > scala request objects. > > > > Thanks, > > > > Jun > > > > > > On Tue, Mar 17, 2015 at 6:08 PM, Gwen Shapira <gshap...@cloudera.com> > wrote: > > > >> Hi, > >> > >> I'm starting this thread for the various questions I run into while > >> refactoring the server to use client requests and responses. > >> > >> Help is appreciated :) > >> > >> First question: LEADER_AND_ISR request and STOP_REPLICA request are > >> unimplemented in the client. > >> > >> Do we want to implement them as part of this refactoring? > >> Or should we continue using the scala implementation for those? > >> > >> Gwen > >> >