You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "AndrewJSchofield (via GitHub)" <gi...@apache.org> on 2025/04/04 17:17:48 UTC
[PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
AndrewJSchofield opened a new pull request, #19378:
URL: https://github.com/apache/kafka/pull/19378
This PR removes the unstable API flag for the KIP-932 RPCs.
The 4 RPCs which were exposed for the early access release in AK 4.0 are stabilised at v1. This is because the RPCs have evolved over time and AK 4.0 clients are not compatible with AK 4.1 brokers. By stabilising at v1, the API version checks prevent incompatible communication and server-side exceptions when trying to parse the requests from the older clients.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037776176
##########
clients/src/main/java/org/apache/kafka/common/requests/AlterShareGroupOffsetsRequest.java:
##########
@@ -41,7 +41,7 @@ public static class Builder extends AbstractRequest.Builder<AlterShareGroupOffse
private final AlterShareGroupOffsetsRequestData data;
public Builder(AlterShareGroupOffsetsRequestData data) {
- this(data, true);
+ this(data, false);
}
public Builder(AlterShareGroupOffsetsRequestData data, boolean enableUnstableLastVersion) {
Review Comment:
Ah, yes. I thought that was a standard thing, but I see that it's not necessary. Removed.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037272992
##########
clients/src/main/java/org/apache/kafka/common/requests/AlterShareGroupOffsetsRequest.java:
##########
@@ -41,7 +41,7 @@ public static class Builder extends AbstractRequest.Builder<AlterShareGroupOffse
private final AlterShareGroupOffsetsRequestData data;
public Builder(AlterShareGroupOffsetsRequestData data) {
- this(data, true);
+ this(data, false);
}
public Builder(AlterShareGroupOffsetsRequestData data, boolean enableUnstableLastVersion) {
Review Comment:
Shouldn't we get rid of `enableUnstableLastVersion` constructor i.e. `Builder(AlterShareGroupOffsetsRequestData data, boolean enableUnstableLastVersion)`, default is `false` for `enableUnstableLastVersion` in `AbstractRequest`. So only method would be:
```
public Builder(AlterShareGroupOffsetsRequestData data) {
super(ApiKeys.ALTER_SHARE_GROUP_OFFSETS);
this.data = data;
}
##########
clients/src/main/resources/common/message/ShareAcknowledgeRequest.json:
##########
@@ -18,34 +18,33 @@
"type": "request",
"listeners": ["broker"],
"name": "ShareAcknowledgeRequest",
- "validVersions": "0",
- "flexibleVersions": "0+",
- // The ShareAcknowledgeRequest API is added as part of KIP-932 and is still under
- // development. Hence, the API is not exposed by default by brokers unless
- // explicitly enabled.
- "latestVersionUnstable": true,
+ // Version 0 was used for early access of KIP-932 in Apache Kafka 4.0 but removed in Apacke Kafka 4.1.
+ //
+ // Version 1 is the initial stable version (KIP-932).
+ "validVersions": "1",
Review Comment:
This will ensure that version 4.0 client requests are not accepted by 4.1 broker, correct? Then why do we need to update the version number for individual fields. They should be on 0+ itself and fields changed/added from 4.0 -> 4.1 should be on 1+, isn't it?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037882487
##########
clients/src/main/resources/common/message/ShareAcknowledgeResponse.json:
##########
@@ -17,7 +17,10 @@
"apiKey": 79,
"type": "response",
"name": "ShareAcknowledgeResponse",
- "validVersions": "0",
+ // Version 0 was used for early access of KIP-932 in Apache Kafka 4.0 but removed in Apacke Kafka 4.1.
+ //
+ // Version 1 is the initial stable version (KIP-932).
+ "validVersions": "1",
"flexibleVersions": "0+",
Review Comment:
Should this be 1+ as well. Will this condition fail: https://github.com/apache/kafka/blob/c65a161cd8b4ef0bbc370be735a6d1dd9b413381/generator/src/main/java/org/apache/kafka/message/checker/EvolutionVerifier.java#L53?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037938413
##########
clients/src/main/resources/common/message/ShareAcknowledgeResponse.json:
##########
@@ -17,7 +17,10 @@
"apiKey": 79,
"type": "response",
"name": "ShareAcknowledgeResponse",
- "validVersions": "0",
+ // Version 0 was used for early access of KIP-932 in Apache Kafka 4.0 but removed in Apacke Kafka 4.1.
+ //
+ // Version 1 is the initial stable version (KIP-932).
+ "validVersions": "1",
"flexibleVersions": "0+",
Review Comment:
I think it's permissible for flexible versions to be broader than valid versions (see `EvolutionVerifierTest`). I can do some more digging, but it doesn't seem wrong to me at this point.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037973763
##########
clients/src/main/resources/common/message/ShareAcknowledgeResponse.json:
##########
@@ -17,7 +17,10 @@
"apiKey": 79,
"type": "response",
"name": "ShareAcknowledgeResponse",
- "validVersions": "0",
+ // Version 0 was used for early access of KIP-932 in Apache Kafka 4.0 but removed in Apacke Kafka 4.1.
+ //
+ // Version 1 is the initial stable version (KIP-932).
+ "validVersions": "1",
"flexibleVersions": "0+",
Review Comment:
The code I pointed actually checks if field within the request has flexibleVersions then it should be within validVersions. Though it seems stange that why the same check do not exists for toplevel flexibleVersions.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037944799
##########
clients/src/main/resources/common/message/ShareFetchRequest.json:
##########
@@ -48,6 +47,8 @@
"about": "The partitions to fetch.", "fields": [
{ "name": "PartitionIndex", "type": "int32", "versions": "0+",
"about": "The partition index." },
+ { "name": "PartitionMaxBytes", "type": "int32", "versions": "0",
+ "about": "The maximum bytes to fetch from this partition. 0 when only acknowledgement with no fetching is required. See KIP-74 for cases where this limit may not be honored." },
Review Comment:
Yeah, so we won't remove it just deprecate 👍
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037892822
##########
clients/src/main/resources/common/message/ShareFetchRequest.json:
##########
@@ -48,6 +47,8 @@
"about": "The partitions to fetch.", "fields": [
{ "name": "PartitionIndex", "type": "int32", "versions": "0+",
"about": "The partition index." },
+ { "name": "PartitionMaxBytes", "type": "int32", "versions": "0",
+ "about": "The maximum bytes to fetch from this partition. 0 when only acknowledgement with no fetching is required. See KIP-74 for cases where this limit may not be honored." },
Review Comment:
Query: Can't we remove it from Request itself?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037929838
##########
clients/src/main/resources/common/message/ShareFetchRequest.json:
##########
@@ -48,6 +47,8 @@
"about": "The partitions to fetch.", "fields": [
{ "name": "PartitionIndex", "type": "int32", "versions": "0+",
"about": "The partition index." },
+ { "name": "PartitionMaxBytes", "type": "int32", "versions": "0",
+ "about": "The maximum bytes to fetch from this partition. 0 when only acknowledgement with no fetching is required. See KIP-74 for cases where this limit may not be honored." },
Review Comment:
It was in v0 (AK 4.0).
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield merged PR #19378:
URL: https://github.com/apache/kafka/pull/19378
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037943935
##########
clients/src/main/resources/common/message/ShareAcknowledgeResponse.json:
##########
@@ -17,7 +17,10 @@
"apiKey": 79,
"type": "response",
"name": "ShareAcknowledgeResponse",
- "validVersions": "0",
+ // Version 0 was used for early access of KIP-932 in Apache Kafka 4.0 but removed in Apacke Kafka 4.1.
+ //
+ // Version 1 is the initial stable version (KIP-932).
+ "validVersions": "1",
"flexibleVersions": "0+",
Review Comment:
Yeah, I tried to run using console-share-consumer and it works. But looking at the code there seems a check. And I don't see any request/response which has flexibleVersions lesser than validVersion. I am digging as well.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "AndrewJSchofield (via GitHub)" <gi...@apache.org>.
AndrewJSchofield commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037371932
##########
clients/src/main/resources/common/message/ShareAcknowledgeRequest.json:
##########
@@ -18,34 +18,33 @@
"type": "request",
"listeners": ["broker"],
"name": "ShareAcknowledgeRequest",
- "validVersions": "0",
- "flexibleVersions": "0+",
- // The ShareAcknowledgeRequest API is added as part of KIP-932 and is still under
- // development. Hence, the API is not exposed by default by brokers unless
- // explicitly enabled.
- "latestVersionUnstable": true,
+ // Version 0 was used for early access of KIP-932 in Apache Kafka 4.0 but removed in Apacke Kafka 4.1.
+ //
+ // Version 1 is the initial stable version (KIP-932).
+ "validVersions": "1",
Review Comment:
I just went for a clean break at "1" rather than trying to document the original "0" versions as well. For ShareFetchRequest in particular, things have move a bit between 4.0 and 4.1.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19057: Stabilize KIP-932 RPCs for AK 4.1 [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on code in PR #19378:
URL: https://github.com/apache/kafka/pull/19378#discussion_r2037943935
##########
clients/src/main/resources/common/message/ShareAcknowledgeResponse.json:
##########
@@ -17,7 +17,10 @@
"apiKey": 79,
"type": "response",
"name": "ShareAcknowledgeResponse",
- "validVersions": "0",
+ // Version 0 was used for early access of KIP-932 in Apache Kafka 4.0 but removed in Apacke Kafka 4.1.
+ //
+ // Version 1 is the initial stable version (KIP-932).
+ "validVersions": "1",
"flexibleVersions": "0+",
Review Comment:
Yeah, I tried to run using console-share-consumer and it works. But looking at the code there seems a check. And I don't see any request/response which has flexibleVersions lesser than validVersion. May be it's only when there are taggedFields, I am digging as well.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org