You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/01 23:34:54 UTC
[GitHub] [kafka] cmccabe opened a new pull request #10804: KAFKA-12877: fix KRPC files with missing flexibleVersions annotations
cmccabe opened a new pull request #10804:
URL: https://github.com/apache/kafka/pull/10804
Some KRPC files do not specify their flexibleVersions. Unfortunately,
in this case, we default to not supporting any flexible versions. This
is a poor default, since the flexible format is both more efficient
(usually) and flexible.
Make flexibleVersions explicit and disallow setting anything except
"0+" on new RPC, metadata, and storage records.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #10804: KAFKA-12877: fix KRPC files with missing flexibleVersions annotations
Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #10804:
URL: https://github.com/apache/kafka/pull/10804#issuecomment-854102079
The patch makes sense to me. One alternative to consider is to invert the flexible version specification. Basically get rid of "flexibleVersions" and instead add a "legacyVersions" which marks the explicit versions which do not support flexible versions. Then new types get flexible versions by default with just the "versions" tag.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji edited a comment on pull request #10804: KAFKA-12877: fix KRPC files with missing flexibleVersions annotations
Posted by GitBox <gi...@apache.org>.
hachikuji edited a comment on pull request #10804:
URL: https://github.com/apache/kafka/pull/10804#issuecomment-854102079
The patch makes sense to me. One alternative to consider is to inverting the flexible version specification. Basically get rid of "flexibleVersions" and instead add a "legacyVersions" which marks the explicit versions which do not support flexible versions. Then new types get flexible versions by default with just the "versions" tag.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #10804: KAFKA-12877: fix KRPC files with missing flexibleVersions annotations
Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #10804:
URL: https://github.com/apache/kafka/pull/10804#issuecomment-854102079
The patch makes sense to me. One alternative to consider is to invert the flexible version specification. Basically get rid of "flexibleVersions" and instead add a "legacyVersions" which marks the explicit versions which do not support flexible versions. Then new types get flexible versions by default with just the "versions" tag.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji edited a comment on pull request #10804: KAFKA-12877: fix KRPC files with missing flexibleVersions annotations
Posted by GitBox <gi...@apache.org>.
hachikuji edited a comment on pull request #10804:
URL: https://github.com/apache/kafka/pull/10804#issuecomment-854102079
The patch makes sense to me. One alternative to consider is to inverting the flexible version specification. Basically get rid of "flexibleVersions" and instead add a "legacyVersions" which marks the explicit versions which do not support flexible versions. Then new types get flexible versions by default with just the "versions" tag.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #10804: KAFKA-12877: Make flexibleVersions mandatory
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #10804:
URL: https://github.com/apache/kafka/pull/10804#issuecomment-861030811
Seems like there was a Jenkins infra issue on the last test run.
```
[2021-06-14T20:15:25.946Z] Execution failed for task ':streams:streams-scala:compileScala'.
[2021-06-14T20:15:25.946Z] > Timeout waiting to lock zinc-1.3.5_2.13.6_16 compiler cache (/home/jenkins/.gradle/caches/7.0.2/zinc-1.3.5_2.13.6_16). It is currently in use by another Gradle instance.
[2021-06-14T20:15:25.946Z] Owner PID: 36970
[2021-06-14T20:15:25.946Z] Our PID: 37256
[2021-06-14T20:15:25.946Z] Owner Operation:
[2021-06-14T20:15:25.946Z] Our operation:
[2021-06-14T20:15:25.946Z] Lock file: /home/jenkins/.gradle/caches/7.0.2/zinc-1.3.5_2.13.6_16/zinc-1.3.5_2.13.6_16.lock
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe commented on pull request #10804: KAFKA-12877: Make flexibleVersions mandatory
Posted by GitBox <gi...@apache.org>.
cmccabe commented on pull request #10804:
URL: https://github.com/apache/kafka/pull/10804#issuecomment-860882713
OK, I reconsidered this a bit. I think we DON'T want to make incompatible changes if we can avoid it... such as changing the default from NONE to ALL.
There are a few reasons for this. One is that we eventually want to have a verifier tool that checks that changes between message files shipped with version X and those shipped with version X+1 are compatible. This is a lot easier if we refrain from gratuitous compatibility breaks.
Another is that there are external projects that are looking at these JSON files now. There's no formal contract or anything, but it's just nicer for the ecosystem not to break things.
Finally, downstream forks of Kafka may have a difficult time if we change defaults around like this. For example, if someone had an in-house fork of Kafka with some new messages, changing around defaults could cause a compatibility break for them. It's better not to have to worry about this.
For all these reasons, I think it's best just to require explicitly spelling out `flexibleVersions` for now. It's a little extra boilerplate but not that bad in the grand scheme of things. Maybe eventually we can have more versioning in the JSON file that lets us elide some of this, but for now I think this is the best way to go. cc @hachikuji
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] cmccabe merged pull request #10804: KAFKA-12877: Make flexibleVersions mandatory
Posted by GitBox <gi...@apache.org>.
cmccabe merged pull request #10804:
URL: https://github.com/apache/kafka/pull/10804
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org