PartitionData

[Jay Kreps <ja...@gmail.com>]

Hey Guys,

The request format in 0.8 has drifted a bit from the proposal (
https://cwiki.apache.org/confluence/display/KAFKA/New+Wire+Format+Proposal).
A lot of this was new fields that were needed. But some oddities have
slipped in.

For example we are reusing PartitionData.scala in both the produce request
and the fetch response. This seems like clever code reuse, but in reality
it changes the protocol to add a bunch of non-sensical fields into the
request format. For example in a request one now has to specify a error
code, and initial offset, and a high water mark!

Is this intentional? I recommend we change this before the release.
Thoughts?

-Jay

Re: PartitionData

[Jun Rao <ju...@gmail.com>]

Decoupling the PartitionData btw producer and fetch may not be a bad idea,
especially the way PartitionData is sent to socket is kind of different
(for fetch, the sendfile api is used).

Thanks,

Jun

On Wed, Sep 12, 2012 at 9:32 AM, Jay Kreps <ja...@gmail.com> wrote:

> Hey Guys,
>
> The request format in 0.8 has drifted a bit from the proposal (
> https://cwiki.apache.org/confluence/display/KAFKA/New+Wire+Format+Proposal
> ).
> A lot of this was new fields that were needed. But some oddities have
> slipped in.
>
> For example we are reusing PartitionData.scala in both the produce request
> and the fetch response. This seems like clever code reuse, but in reality
> it changes the protocol to add a bunch of non-sensical fields into the
> request format. For example in a request one now has to specify a error
> code, and initial offset, and a high water mark!
>
> Is this intentional? I recommend we change this before the release.
> Thoughts?
>
> -Jay
>

Re: PartitionData

[Joel Koshy <jj...@gmail.com>]

Yes - something along those lines. We can either do it in a separate jira
or as part of KAFKA-391 since I'm already touching that code.

Thanks,

Joel

On Wed, Sep 12, 2012 at 9:59 AM, Joe Stein <cr...@gmail.com> wrote:

> do want to create a ProducerRequestData and FetchResponseData?
>
> i'll open a ticket
>
> On Wed, Sep 12, 2012 at 12:40 PM, Joel Koshy <jj...@gmail.com> wrote:
>
> > That's a good point. I did notice that while working on KAFKA-391. The
> idea
> > behind it must have been that both producer and fetch requests deal with
> > actual partition data. However, the hw and error code don't make sense on
> > the request side. I can include this change as part of KAFKA-391 if that
> > makes sense.
> >
> > Joel
> >
> > On Wed, Sep 12, 2012 at 9:32 AM, Jay Kreps <ja...@gmail.com> wrote:
> >
> > > Hey Guys,
> > >
> > > The request format in 0.8 has drifted a bit from the proposal (
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/New+Wire+Format+Proposal
> > > ).
> > > A lot of this was new fields that were needed. But some oddities have
> > > slipped in.
> > >
> > > For example we are reusing PartitionData.scala in both the produce
> > request
> > > and the fetch response. This seems like clever code reuse, but in
> reality
> > > it changes the protocol to add a bunch of non-sensical fields into the
> > > request format. For example in a request one now has to specify a error
> > > code, and initial offset, and a high water mark!
> > >
> > > Is this intentional? I recommend we change this before the release.
> > > Thoughts?
> > >
> > > -Jay
> > >
> >
>
>
>
> --
>
> /*
> Joe Stein
> http://www.linkedin.com/in/charmalloc
> Twitter: @allthingshadoop <http://www.twitter.com/allthingshadoop>
> */
>

Re: PartitionData

[Joe Stein <cr...@gmail.com>]

do want to create a ProducerRequestData and FetchResponseData?

i'll open a ticket

On Wed, Sep 12, 2012 at 12:40 PM, Joel Koshy <jj...@gmail.com> wrote:

> That's a good point. I did notice that while working on KAFKA-391. The idea
> behind it must have been that both producer and fetch requests deal with
> actual partition data. However, the hw and error code don't make sense on
> the request side. I can include this change as part of KAFKA-391 if that
> makes sense.
>
> Joel
>
> On Wed, Sep 12, 2012 at 9:32 AM, Jay Kreps <ja...@gmail.com> wrote:
>
> > Hey Guys,
> >
> > The request format in 0.8 has drifted a bit from the proposal (
> >
> https://cwiki.apache.org/confluence/display/KAFKA/New+Wire+Format+Proposal
> > ).
> > A lot of this was new fields that were needed. But some oddities have
> > slipped in.
> >
> > For example we are reusing PartitionData.scala in both the produce
> request
> > and the fetch response. This seems like clever code reuse, but in reality
> > it changes the protocol to add a bunch of non-sensical fields into the
> > request format. For example in a request one now has to specify a error
> > code, and initial offset, and a high water mark!
> >
> > Is this intentional? I recommend we change this before the release.
> > Thoughts?
> >
> > -Jay
> >
>



-- 

/*
Joe Stein
http://www.linkedin.com/in/charmalloc
Twitter: @allthingshadoop <http://www.twitter.com/allthingshadoop>
*/

Re: PartitionData

[Joel Koshy <jj...@gmail.com>]

That's a good point. I did notice that while working on KAFKA-391. The idea
behind it must have been that both producer and fetch requests deal with
actual partition data. However, the hw and error code don't make sense on
the request side. I can include this change as part of KAFKA-391 if that
makes sense.

Joel

On Wed, Sep 12, 2012 at 9:32 AM, Jay Kreps <ja...@gmail.com> wrote:

> Hey Guys,
>
> The request format in 0.8 has drifted a bit from the proposal (
> https://cwiki.apache.org/confluence/display/KAFKA/New+Wire+Format+Proposal
> ).
> A lot of this was new fields that were needed. But some oddities have
> slipped in.
>
> For example we are reusing PartitionData.scala in both the produce request
> and the fetch response. This seems like clever code reuse, but in reality
> it changes the protocol to add a bunch of non-sensical fields into the
> request format. For example in a request one now has to specify a error
> code, and initial offset, and a high water mark!
>
> Is this intentional? I recommend we change this before the release.
> Thoughts?
>
> -Jay
>

Re: PartitionData

[Jun Rao <ju...@gmail.com>]

Magnus,

Thanks for the feedback. Commented on the wiki.

Jun

On Thu, Sep 13, 2012 at 12:47 AM, Magnus Edenhill <ma...@edenhill.se>wrote:

> 2012/9/12 Jay Kreps <ja...@gmail.com>
>
> > Hey Guys,
> >
> > The request format in 0.8 has drifted a bit from the proposal (
> >
> https://cwiki.apache.org/confluence/display/KAFKA/New+Wire+Format+Proposal
> > ).
> > A lot of this was new fields that were needed. But some oddities have
> > slipped in.
> >
>
> I followed this link and didnt pay much attention to the last edit and
> reply dates, so I put some feedback on there regarding some of the proposed
> fields.
> Then I noticed the 7 month difference... but if its not too late, have a
> look.
>
> Having some form of handshake on connection establishment is a good way to
> allow for future non-intrusive extensions that dont require protocol
> version bumps, each side may advertise their supported extensions/features,
> such as authentication or whatever it may be.
>
> Regards,
> Magnus
>

Re: PartitionData

[Magnus Edenhill <ma...@edenhill.se>]

2012/9/12 Jay Kreps <ja...@gmail.com>

> Hey Guys,
>
> The request format in 0.8 has drifted a bit from the proposal (
> https://cwiki.apache.org/confluence/display/KAFKA/New+Wire+Format+Proposal
> ).
> A lot of this was new fields that were needed. But some oddities have
> slipped in.
>

I followed this link and didnt pay much attention to the last edit and
reply dates, so I put some feedback on there regarding some of the proposed
fields.
Then I noticed the 7 month difference... but if its not too late, have a
look.

Having some form of handshake on connection establishment is a good way to
allow for future non-intrusive extensions that dont require protocol
version bumps, each side may advertise their supported extensions/features,
such as authentication or whatever it may be.

Regards,
Magnus