rondagostino commented on code in PR #14290:
URL: https://github.com/apache/kafka/pull/14290#discussion_r1344732697
##########
clients/src/main/resources/common/message/BrokerRegistrationRequest.json:
##########
@@ -53,6 +53,8 @@
{ "name": "Rack", "type": "string", "versions": "0+", "nullableVersions":
"0+",
"about": "The rack which this broker is in." },
{ "name": "IsMigratingZkBroker", "type": "bool", "versions": "1+",
"default": "false",
- "about": "If the required configurations for ZK migration are present,
this value is set to true" }
+ "about": "If the required configurations for ZK migration are present,
this value is set to true" },
+ { "name": "LogDirs", "type": "[]uuid", "versions": "2+",
+ "about": "Log directories configured in this broker which are
available." }
Review Comment:
Noting that `ControllerRegistrationRequest` was just added via
https://github.com/apache/kafka/pull/14306. Do we want to add these there as
well? We don't use ISR on controllers, but it might be worthwhile to store the
data anyway. Could always leave it out for now and keep it in mind for later.
cc @cmccabe
##########
generator/src/main/java/org/apache/kafka/message/FieldSpec.java:
##########
@@ -157,11 +157,6 @@ private void checkTagInvariants() {
this.taggedVersions + ", which is not open-ended.
taggedVersions must " +
"be either none, or an open-ended range (that ends with a
plus sign).");
}
- if
(!this.taggedVersions.intersect(this.versions).equals(this.taggedVersions)) {
- throw new RuntimeException("Field " + name + " specifies
taggedVersions " +
- this.taggedVersions + ", and versions " + this.versions +
". " +
- "taggedVersions must be a subset of versions.");
- }
Review Comment:
I see that without this we get this error when trying to process the json:
```
Exception in thread "main" java.lang.RuntimeException: Exception while
processing src/main/resources/common/metadata/PartitionChangeRecord.json
at
org.apache.kafka.message.MessageGenerator.processDirectories(MessageGenerator.java:248)
at
org.apache.kafka.message.MessageGenerator.main(MessageGenerator.java:367)
Caused by: com.fasterxml.jackson.databind.exc.ValueInstantiationException:
Cannot construct instance of `org.apache.kafka.message.FieldSpec`, problem:
Field Replicas specifies taggedVersions 0+, and versions 0. taggedVersions
must be a subset of versions.
at [Source: (File); line: 36, column: 82] (through reference chain:
org.apache.kafka.message.MessageSpec["fields"]->java.util.ArrayList[4])
```
However, if we try to specify it with `"taggedVersions": "0"`, like this:
```
{ "name": "Replicas", "type": "[]int32", "default": "null",
"entityType": "brokerId",
"versions": "0", "nullableVersions": "0+", "taggedVersions": "0",
"tag": 2,
"about": "null if the replicas didn't change; the new replicas
otherwise." },
```
We get a different error:
```
Exception in thread "main" java.lang.RuntimeException: Exception while
processing src/main/resources/common/metadata/PartitionChangeRecord.json
at
org.apache.kafka.message.MessageGenerator.processDirectories(MessageGenerator.java:248)
at
org.apache.kafka.message.MessageGenerator.main(MessageGenerator.java:367)
Caused by: com.fasterxml.jackson.databind.exc.ValueInstantiationException:
Cannot construct instance of `org.apache.kafka.message.FieldSpec`, problem:
Field Replicas specifies taggedVersions 0, which is not open-ended.
taggedVersions must be either none, or an open-ended range (that ends with a
plus sign).
at [Source: (File); line: 36, column: 82] (through reference chain:
org.apache.kafka.message.MessageSpec["fields"]->java.util.ArrayList[4])
```
The processing code is assuming that a tagged field can never be removed. I
do not understand why this constraint holds. @cmccabe could you comment?
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
