Guys,
I've just uploaded the patch which aligns versionId in getErrorResponse.
(https://issues.apache.org/jira/browse/KAFKA-2195)
Would be great if someone could review it because it's a blocker for
other requests including MetadataRequest which I have to update to V1 for
KIP-4.
Thanks,
Andrii Bi
No worries, I think it's hard to foresee all nuances before
actually implementing/writing code.
I also have a feeling there will be some other issues
with requests versioning so I plan to finish end-to-end
MetadataRequest_V1 to ensure we don't miss anything
in terms of AbstractRequest/Response API,
Sorry for the confusion regarding errorResponses...
On Mon, May 18, 2015 at 9:10 PM, Andrii Biletskyi <
andrii.bilets...@stealth.ly> wrote:
> Jun,
>
> Thanks for the explanation. I believe my understanding is close to what
> you have written. I see, I still think that this approach is
> somewhat
Hi, Andrii,
Good point on the constructor for MetadataResponse. Perhaps in V2 of
MetadataResponse, we can add a new constructor with both cluster and a
topicConfig map. Currently, cluster doesn't really need topic configs and
we can leave it as it is now.
Thanks,
Jun
On Mon, May 18, 2015 at 11:
Jun,
Thanks for the explanation. I believe my understanding is close to what
you have written. I see, I still think that this approach is
somewhat limiting
(what if you add field of type int in V1 and then remove another field of
type int in V2 - method overloading for V0 and V2 constructors will
Andri,
Let me clarify a bit how things work now. You can see if this fits your
need or if it can be improved. If you look at OffsetCommitRequest, our
convention is the following.
1. The request object can be constructed from a set of required fields. The
client typically constructs a request obje
Hi all,
I started working on it and it seems we are going the wrong way.
So it appears we need to distinguish constructors by versions in
request/response (so we can set correct schema).
Request/Response classes will look like:
class SomeRequest extends AbstractRequest {
SomeRequest(versionId,
Okay,
I can pick that. I'll create sub-task under KAFKA-2044.
Thanks,
Andrii Biletskyi
On Fri, May 15, 2015 at 4:27 PM, Gwen Shapira wrote:
> Agree that you need version in getErrorResponse too (so you'll get the
> correct error), which means you'll need to add versionId to constructors of
> ev
Agree that you need version in getErrorResponse too (so you'll get the
correct error), which means you'll need to add versionId to constructors of
every response object...
You'll want to keep two interfaces, one with version and one with
CURR_VERSION as default, so you won't need to modify every s
Correct, I think we are on the same page.
This way we can fix RequestChannel part (where it uses
AbstractRequest.getRequest)
But would it be okay to add versionId to AbstractRequest.getErrorResponse
signature too?
I'm a bit lost with all those Abstract... objects hierarchy and not sure
whether it'
I agree, we currently don't handle versions correctly when de-serializing
into java objects. This will be an isssue for every req/resp we move to use
the java objects.
It looks like this requires:
1. Add versionId parameter to all parse functions in Java req/resp objects
2. Modify getRequest to pa
Gwen,
I didn't find this in answers above so apologies if this was discussed.
It's about the way we would like to handle request versions.
As I understood from Jun's answer we generally should try using the same
java object while evolving the request. I believe the only example of
evolved
request
OK, I posted a working patch on KAFKA-2044 and
https://reviews.apache.org/r/32459/diff/.
There are few decisions there than can be up to discussion (factory method
on AbstractRequestResponse, the new handleErrors in request API), but as
far as support for o.a.k.common requests in core goes, it doe
Hi,
I uploaded a (very) preliminary patch with my idea.
One thing thats missing:
RequestResponse had handleError method that all requests implemented,
typically generating appropriate error Response for the request and sending
it along. Its used by KafkaApis to handle all protocol errors for val
I think what you are saying is that in RequestChannel, we can start
generating header/body for new request types and leave requestObj null. For
existing requests, header/body will be null initially. Gradually, we can
migrate each type of requests by populating header/body, instead of
requestObj. Th
I'm thinking of a different approach, that will not fix everything, but
will allow adding new requests without code duplication (and therefore
unblock KIP-4):
RequestChannel.request currently takes a buffer and parses it into an "old"
request object. Since the objects are byte-compatibly, we shoul
The transferTo stuff is really specialized for sending a fetch response
from a broker. Since we can't get rid of the scala FetchResponse
immediately, we can probably keep the way that fetch responses are sent
(through FetchResponseSend) right now until the protocol definition is
extended.
Thanks,
Ack, yeah, forgot about that.
It's not just a difference of wrappers. The server side actually sends the
bytes lazily using FileChannel.transferTo. We need to make it possible to
carry over that optimization. In some sense what we want to be able to do
is set a value to a Send instead of a ByteBuf
In case anyone is still following this thread, I need a bit of help :)
The old FetchResponse.PartitionData included a MessageSet object.
The new FetchResponse.PartitionData includes a ByteBuffer.
However, when we read from logs, we return a MessageSet, and as far as I
can see, these can't be conv
Note: I'm also treating ZkUtils as if it was a public API (i.e. converting
objects that are returned into o.a.k.common equivalents but not changing
ZkUtils itself).
I know its not public, but I suspect I'm not the only developer here who
has tons of external code that uses it.
Gwen
On Wed, Mar 18
We can't rip them out completely, unfortunately - the SimpleConsumer uses them.
So we'll need conversion at some point. I'll try to make the
conversion point "just before hitting a public API that we can't
modify", and hopefully it won't look too arbitrary.
On Wed, Mar 18, 2015 at 5:24 PM, Jay
I think either approach is okay in the short term. However our goal should
be to eventually get rid of that duplicate code, so if you are up for just
ripping and cutting that may get us there sooner.
-Jay
On Wed, Mar 18, 2015 at 5:19 PM, Gwen Shapira wrote:
> Thanks!
>
> Another clarification:
Thanks!
Another clarification:
The Common request/responses use slightly different infrastructure
objects: Node instead of Broker, TopicPartition instead of
TopicAndPartition and few more.
I can write utilities to convert Node to Broker to minimize the scope
of the change.
Or I can start replacin
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.
3. The way that things are working right now is that there is a single
request object for all versions. The layout of the request object always
corresponds to the latest version. Under normal version evolution, the
request object should be able to be constructed the binary of any version.
For examp
See inline responses:
On Wed, Mar 18, 2015 at 2:26 PM, Jay Kreps 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
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/b
Yes, that looks reasonable. Then, in KafkaApi, when we get into the
handling of a specific request, we can construct the specific request
object from struct.
Thanks,
Jun
On Wed, Mar 18, 2015 at 11:11 AM, Gwen Shapira
wrote:
> Hi Jun,
>
> I was taking a slightly different approach. Let me know
OK, Andrii clarified that we need KAFKA-1927 before KIP-4 for the
infrastructure for using the common request/response classes in core.
Jun, when you got a moment, please confirm if the approach I'm taking
is acceptable, or if you see issues that I'm missing.
On Wed, Mar 18, 2015 at 11:33 AM, G
Looking at the SimpleConsumer, we need to leave in place:
ConsumerMetadataResponse
FetchRequest / FetchResponse
OffsetFetchRequest / OffsetFetchResponse
OffsetCommitRequest / OffsetCommitResponse
OffsetRequest / OffsetResponse
TopicMetadata
TopicMetadataRequest / TopicMetadataResponse
Specificall
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:
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
Yes,
I was talking about TopicMetadataRequest.scala (TMR), which is
MetadataRequest
in java definitions.
None of the mentioned above messages (StopReplica etc) are required in
KIP-4.
Sorry, if I misled someone.
Thanks,
Andrii Biletskyi
On Wed, Mar 18, 2015 at 5:42 PM, Gwen Shapira wrote:
> Sor
Sorry, Andrii, I'm a bit confused.
The following request/response pairs are currently not implemented in
the client:
StopReplica, LeaderAndIsr, UpdateMetadata, ControlledShutdown.
Does KIP-4 require any of these?
Also, what's TMR?
Gwen
On Wed, Mar 18, 2015 at 4:07 AM, Andrii Biletskyi
wrote:
Gwen,
Thanks for bringing this up!
Regarding UpdateMetadata in KIP-4 - no it shouldn't be used in Admin CLI,
its internal server message. We will probably use TMR there (depends how
generic re-routing facility goes) but TMR is already used in NetworkClient,
so
I believe there are no doubts about i
>
> How about keeping those server-side for now, since there's no duplication?
> We can move them over in a follow-up jira.
+1. The work involved in this refactoring is pretty sizable. I would be ok
with splitting it in 2 JIRAs - one that converts the duplicated ones over
first and another that c
Never mind, it is implemented. Its just called LIST_OFFSETS.
On Tue, Mar 17, 2015 at 6:27 PM, Gwen Shapira wrote:
> Thanks Jiangjie!
>
> Another, more questionable missing-in-action:
>
> Client API had offset fetch request and offset commit requests. I
> understand those :)
> The Server API als
Thanks Jiangjie!
Another, more questionable missing-in-action:
Client API had offset fetch request and offset commit requests. I
understand those :)
The Server API also has OffsetRequest, which returns offset
corresponding to a specific time stamp. Its used by consumers.
This sounds like somethi
I think those two requests are only used by controller to broker
communication. Not sure if client side will need them in KIP-4, unlikely I
guess.
Jiangjie (Becket) Qin
On 3/17/15, 6:08 PM, "Gwen Shapira" wrote:
>Hi,
>
>I'm starting this thread for the various questions I run into while
>refact
Same for UPDATE_METADATA. Also, missing in client side. For good reasons :)
How about keeping those server-side for now, since there's no duplication?
We can move them over in a follow-up jira.
Gwen
On Tue, Mar 17, 2015 at 6:08 PM, Gwen Shapira wrote:
> Hi,
>
> I'm starting this thread for the
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 th
41 matches
Mail list logo
