Great.
For (3) yeah I think we should just think through the end-to-end pattern
for these versioned requests since it seems like we will have a number of
them. The serialization code used as you described gets us to the right
Struct which the user would then wrap in something like ProduceRequest.
Presumably there would just be one ProduceRequest that would internally
fill in things like null or otherwise adapt the struct to a usable object.
On the response side we would have the version from the request to use for
correct versioning. On question is whether this is enough or whether we
need to have switches in KafkaApis to do things like
if(produceRequest.version == 3)
// do something
else
// do something else
Basically it would be good to be able to write a quick wiki that was like
"how to add or modify a kafka api" that explained the right way to do all
this.
I don't think any of this necessarily blocks this ticket since at the
moment we don't have tons of versions of requests out there.
-Jay
On Wed, Mar 18, 2015 at 2:50 PM, Gwen Shapira <gshap...@cloudera.com> wrote:
> See inline responses:
>
> On Wed, Mar 18, 2015 at 2:26 PM, Jay Kreps <jay.kr...@gmail.com> wrote:
> > 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.
>
> SimpleConsumer is obviously stuck with the old request/response.
>
> The Producers can be converted to the common request/response without
> breaking compatibility.
> I think we should do this (even though it requires fiddling with
> additional network serialization code), just so we can throw the old
> ProduceRequest away.
>
> Does that make sense?
>
>
> > 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.
>
> Yep.
>
> > 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).
>
> The way I see it working (I just started on this, so I may have gaps):
>
> * Request header contains the version
> * When we read the request, we use ProtoUtils.requestSchema which
> takes version as a parameter and is responsible to give us the right
> Schema, which we use to read the buffer and get the correct struct.
> * KafkaApis handlers have the header, so they can use it to access the
> correct fields, build the correct response, etc.
>
> Does that sound about right?
>
>
> > 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.
>
> I'll make a note to validate that this is the case.
>
> >
> > -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
> >> >>
> >>
>
