[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14165278#comment-14165278
]
Jun Rao commented on KAFKA-1644:
Ok, since this a small patch, I double committed to 0.8.2.
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Improvement
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Fix For: 0.8.2, 0.8.3
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch,
0002-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch,
0003-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14165287#comment-14165287
]
Anton Karamanov commented on KAFKA-1644:
That's great news, thanks.
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Improvement
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Fix For: 0.8.2, 0.8.3
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch,
0002-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch,
0003-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14161187#comment-14161187
]
Jun Rao commented on KAFKA-1644:
Perhaps it's better to throw a UnsupportedOperationExceptions in writeTo()?
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Improvement
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch,
0002-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14159758#comment-14159758
]
Jun Rao commented on KAFKA-1644:
Alexey,
My only concern of the patch is that it gives people the impression that
FetchResponse.writeTo() is supported in the same way as other
requests/responses by serializing the full request/response into a ByteBuffer.
which can be misleading.
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Improvement
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14158114#comment-14158114
]
Alexey Ozeritskiy commented on KAFKA-1644:
--
This patch simplifies writing clients. Now we have to use the following ugly
code:
{code:java}
if (id == ResponseKeys.FetchKey) {
val response = FetchResponse.readFrom(contentBuffer)
listener ! FetchAnswer(response)
} else {
val response = ResponseKeys.deserializerForKey(id)(contentBuffer)
listener ! Answer(response)
}
{code}
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Bug
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14156149#comment-14156149
]
Anton Karamanov commented on KAFKA-1644:
Jun, no {{FetchResponseSend}} still uses sendfile logic.
{{FetchResponse.writeTo()}} only writes a fixed sized fetch message header to
buffer.
{code}
- buffer.putInt(size)
- buffer.putInt(fetchResponse.correlationId)
- buffer.putInt(fetchResponse.dataGroupedByTopic.size) // topic count
+ fetchResponse.writeTo(buffer)
{code}
Guozhang, indeed it is not, it's priority can be lowered if necessary. But it's
a pretty simple change too.
The problem you are describing with {{FetchResponse.writeTo}} not being
semantically consistent with that of other API messages is the very same I've
mentioned as my concern. However, it seems like an OK compromise to me to use
it for writing fixed-size message header, which depends only on
{{FetchResponse}} members ({{correlationId}} and size of
{{dataGroupedByTopic}}) instead of making it to do nothing at all just to
conform to required function signature.
My primary goal for this task is to make {{FetchResponse}} to be a part of
common superclass to simplify client-side message flow control. Maybe another
trait can be introduced without any abstract members, just to mark all Kafka
API messages. Or {{RequestOrResponse}} can be split into {{RequestOrResponse}}
without {{writeTo}} abstract method and something like {{Writable}} trait which
will require to provide it.
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Bug
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14155678#comment-14155678
]
Guozhang Wang commented on KAFKA-1644:
--
Anton, I think Jun's not saying that there is a blocker for doing so, it's just
that since FetchResponse does not use writeTo to write to the socket, but use
FetchReponseSend instead for zero-copy, it may not logically be treated as
inheritance of RequestOrResponse. After looking into your patch I see you are
enforcing this by letting FetchResponseSend to just call FetchResponse.writeTo
instead, which I think is OK if we do have a strong reason for letting
FetchResponse inheriting from RequestOrResponse.
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Bug
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14155903#comment-14155903
]
Jun Rao commented on KAFKA-1644:
Anton,
By letting FetchResponseSend to call FetchResponse.writeTo(), you are no longer
using the sendfile logic, right?
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Bug
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14151423#comment-14151423
]
Anton Karamanov commented on KAFKA-1644:
Does this prevent it from being inherited from {{RequestOrResponse}}? From what
I see there's no real impact on server-side code and it would be really useful
on a client-side. Especially now that [API is completely public
(KAFKA-1640)|KAFKA-1640]).
My only concern is that {{RequestOrResponse}} trait requires to implement
{{writeTo}} method, which wouldn't work for {{FetchResponse}} the same way it
does for other {{RequestOrResponse}}s. In the patch I've provided it only
serves to write a header of a fetch response.
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Bug
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14151136#comment-14151136
]
Jun Rao commented on KAFKA-1644:
The reason that FetchResponse is special is that it uses the sendfile api to
transfer data from the filechannel to a remote socket, which avoids creating a
copy of the data in jvm heap.
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Bug
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (KAFKA-1644) Inherit FetchResponse from RequestOrResponse
[
https://issues.apache.org/jira/browse/KAFKA-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14143112#comment-14143112
]
Anton Karamanov commented on KAFKA-1644:
Here's a
[patch|^0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch].
Inherit FetchResponse from RequestOrResponse
Key: KAFKA-1644
URL: https://issues.apache.org/jira/browse/KAFKA-1644
Project: Kafka
Issue Type: Bug
Reporter: Anton Karamanov
Assignee: Anton Karamanov
Attachments:
0001-KAFKA-1644-Inherit-FetchResponse-from-RequestOrRespo.patch
Unlike all other Kafka API responses {{FetchResponse}} is not a subclass of
RequestOrResponse, which requires handling it as a special case while
processing responses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
