KAFKA-4208: Add Record Headers#2991
Conversation
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
becketqin
left a comment
becketqin
left a comment
There was a problem hiding this comment.
Thanks for the patch. Left some comments. It seems that 0.11.0 upgrade doc has a lot of thing to update.
@hachikuji @ijuma do we plan to update the upgrade doc when KIP-98 is completely done?
There was a problem hiding this comment.
It seems a little verbose to elaborate all the methods of Headers interface in the upgrade doc. See if the following is more concise.
<li>A new Headers interface has been introduced to provide headers read and write access.</li>
<li>ProducerRecord and ConsumerRecord expose the new Headers API via <code>Headers headers()</code> method call.</li>
<li>ExtendedSerializer and ExtendedDeserializer interfaces are introduced to support serialization and deserialization for headers. Headers will be ignored if the configured serializer and deserializer are not the above classes.</li>
There are some useful information in the current description, but may be more suitable for the Headers java doc. In the upgrade doc, it is probably not necessary to explain the details of a newly introduced feature (immutable headers after interceptor), but we do need to explain if there is any change of the existing behaviors.
There was a problem hiding this comment.
To clarify, upgrade.html usually only includes changes with a compatibility impact. At the moment, items that should be part of the release notes are added to the release plan:
https://cwiki.apache.org/confluence/display/KAFKA/Release+Plan+0.11.0.0
Actual documentation of how Headers work, etc. should be added to the Javadoc and potentially to the Kafka documentation (see docs folder).
There was a problem hiding this comment.
So it seems i just need to thin out/be less verbose (prob just take @becketqin improved version). And just add versions on the protocol. will do this.
Javadocs all there already, so should be picked up.
There was a problem hiding this comment.
Can we be more precise on the message format version? e.g. 0.11.0. Same for the descriptions below, Older clients -> clients before 0.11.0
|
@becketqin yes, the JIRA for that is https://issues.apache.org/jira/browse/KAFKA-5019 |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
Update upgrade.html
Update based on review feedback. still need to add protocol / versions.
Adding protocol versions.
michaelandrepearce
force-pushed
the
KIP-82
branch
from
c95a63f to
911d263
Compare
|
rebased to sort conflicts |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
| producer's <code>batch.size</code> configuration.</li> | ||
| <li>GC log rotation is enabled by default, see KAFKA-3754 for details.</li> | ||
| <li>Deprecated constructors of MetricName and Cluster classes have been removed.</li> | ||
| <li>A new Headers interface has been introduced to provide headers read and write access.</li> |
There was a problem hiding this comment.
It seems this may cause confusion between user headers and system headers. Would the following be less ambiguous?
"Added user headers support through a new Headers interface."
minor description twerk, based on feedback.
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Thanks for updating the doc. LGTM. |
Update upgrade.html Raising this now, as KIP-118 is pulled from release as such submitting this without java 8 changes. As per remaining review comment from #2772, updating the upgrade notes. Author: Michael André Pearce <michael.andre.pearce@me.com> Author: Michael Andre Pearce <Michael.Andre.Pearce@me.com> Reviewers: Jiangjie Qin <becket.qin@gmail.com>, Ismael Juma <ismael@juma.me.uk> Closes #2991 from michaelandrepearce/KIP-82
4 participants
Update upgrade.html
Raising this now, as KIP-118 is pulled from release as such submitting this without java 8 changes.
As per remaining review comment from #2772, updating the upgrade notes.