dajac commented on code in PR #18455:
URL: https://github.com/apache/kafka/pull/18455#discussion_r1909927366
##########
coordinator-common/src/main/java/org/apache/kafka/coordinator/common/runtime/CoordinatorRecordSerde.java:
##########
@@ -43,7 +43,7 @@ public abstract class CoordinatorRecordSerde implements
Serializer<CoordinatorRe
@Override
public byte[] serializeKey(CoordinatorRecord record) {
// Record does not accept a null key.
- return MessageUtil.toVersionPrefixedBytes(
+ return MessageUtil.toCoordinatorTypePrefixedBytes(
Review Comment:
> So, it seems to me that toCoordinatorTypePrefixedBytes fixes the version
of the key schema as 0, but using toVersionPrefixedBytes permits other versions
of the record schemas.
This is correct. In other words, toCoordinatorTypePrefixedBytes is used for
keys where the version is always zero and toVersionPrefixedBytes is used for
values where version could be non-zero too.
> However, if I understand correctly, we do not want to use record schemas
above 0 and prefer tagged fields for new fields added to records. What is the
correct way to version records in the future?
Using tagged fields is indeed the preferred way now because this is the only
way to make change backward compatible at the moment. Before we had tagged
fields, we bumped the version of the record when we added fields and we gated
using newer version with the IBP/MV. Once used, there were no way back. It is
still possible to do it this way but it is not recommended. I am not sure if we
will have cases requiring it in the future. Note that we already have non-zero
versions for values hence we support them here. Otherwise, I would have forced
it to zero all the time too.
For the record, we discussed an alternative approach which would follow how
we handle downgrade in the controller. The controller basically generate a new
snapshot with the previous version when it happens. With a compacted topic, it
would require rewriting the entire state to the log and trimming it.
Transactions would need some special care too. Overall, we felt like tagged
field were good enough for now and doing something more complicated was not
justified.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
