You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "ijuma (via GitHub)" <gi...@apache.org> on 2024/12/16 16:56:41 UTC
[PR] KAFKA-XXX: Remove deprecated protocol APIs support (KIP-896) [kafka]
ijuma opened a new pull request, #18218:
URL: https://github.com/apache/kafka/pull/18218
TBD
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1892416566
##########
clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java:
##########
@@ -379,7 +378,7 @@ public void testSerializationSpecialCases() {
checkResponse(createFetchResponse(Errors.FETCH_SESSION_ID_NOT_FOUND, 123), (short) 7);
checkOlderFetchVersions();
// Metadata
- checkRequest(MetadataRequest.Builder.allTopics().build((short) 2));
+ checkRequest(MetadataRequest.Builder.allTopics().build((short) 4)); //FIXME Does version 4 have the special cases?
Review Comment:
This seems fine, version 4 only added `AllowAutoTopicCreation` - it should have whatever special case version 2 had (not ideal that we didn't document that).
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2552103909
> Also, now that I've started reviewing, can you try to avoid force pushes? Otherwise it will reset the "Viewed" state in my review which makes it hard to see what has been reviewed already.
Totally, I usually don't force push once someone starts reviewing. Even in this PR, I haven't actually rewritten history (I added commits) except for one commit I pushed by mistake. It is interesting that the `-f` flag is still captured even when the push doesn't require it. Will make sure I don't use the latter from now on.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma merged PR #18218:
URL: https://github.com/apache/kafka/pull/18218
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1890724339
##########
clients/src/main/resources/common/message/CreateTopicsRequest.json:
##########
@@ -18,6 +18,8 @@
"type": "request",
"listeners": ["zkBroker", "broker", "controller"],
"name": "CreateTopicsRequest",
+ // Versions 0-1 were removed in Apache Kafka 4.0, Version 2 is the new baseline.
Review Comment:
It can be ignored, that is what would have happened if we had used 2.1 as the baseline without the adjustment for third party clients (think of it as a way to describe how we got to the V0-V1 as the final answer).
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1890735689
##########
clients/src/main/resources/common/message/DescribeConfigsResponse.json:
##########
@@ -17,10 +17,11 @@
"apiKey": 32,
"type": "response",
"name": "DescribeConfigsResponse",
+ // Version 0 was removed in Apache Kafka 4.0, Version 1 is the new baseline.
// Version 1 adds ConfigSource and the synonyms.
// Starting in version 2, on quota violation, brokers send out responses before throttling.
// Version 4 enables flexible versions.
- "validVersions": "0-4",
Review Comment:
Good point, this hadn't been mentioned in the changelog and I missed 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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2569363660
No, that's not a discrepancy as far as I can tell. The KIP specifies a particular version of Sarama and links to the relevant code. Can you please check again with that in mind?
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2557624325
@mumrah I think this is ready to go. I updated the PR description with the client versions I tested. One of the consequences of the testing was KAFKA-18334. I adjusted this PR to take it into account and submitted #18288 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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2569012455
Hi @ijuma,
I noticed a discrepancy between this PR and [KIP-896](https://cwiki.apache.org/confluence/x/K5sODg) regarding `DeleteGroups` support:
```
DeleteGroups: none due to clients (Sarama) support - V0 per the baseline
* librdkafka: [V1](https://github.com/confluentinc/librdkafka/blob/v1.8.2/src/rdkafka_request.c#L4147)
* KafkaJS: [V1](https://github.com/tulios/kafkajs/blob/v1.15.0/src/protocol/requests/deleteGroups/v1/request.js#L7)
* Sarama: [V0](https://github.com/Shopify/sarama/blob/v1.29.1/describe_groups_request.go#L21)
* kafka-python: [V1](https://github.com/dpkp/kafka-python/blob/2.0.2/kafka/admin/client.py#L1328)
```
According to the KIP, Sarama only supports V0 of the `DeleteGroups RPC`. However, I found issue (https://github.com/IBM/sarama/issues/2408) which suggests Sarama will upgrade its RPC support. (And it has support v1 after https://github.com/IBM/sarama/pull/2551) If this is the case, should we update the description in KIP-896 (https://cwiki.apache.org/confluence/x/K5sODg)?
WDYT?
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1890777304
##########
clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java:
##########
@@ -42,27 +42,16 @@
public class ProduceRequest extends AbstractRequest {
public static final short LAST_STABLE_VERSION_BEFORE_TRANSACTION_V2 = 11;
- public static Builder forMagic(byte magic, ProduceRequestData data, boolean useTransactionV1Version) {
Review Comment:
Nice to see this record version stuff going away 😄
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -834,81 +832,6 @@ class KafkaApis(val requestChannel: RequestChannel,
}
}
- def maybeConvertFetchedData(tp: TopicIdPartition,
- partitionData: FetchResponseData.PartitionData): FetchResponseData.PartitionData = {
- // We will never return a logConfig when the topic is unresolved and the name is null. This is ok since we won't have any records to convert.
- val logConfig = replicaManager.getLogConfig(tp.topicPartition)
-
- if (logConfig.exists(_.compressionType == BrokerCompressionType.ZSTD) && versionId < 10) {
- trace(s"Fetching messages is disabled for ZStandard compressed partition $tp. Sending unsupported version response to $clientId.")
- FetchResponse.partitionResponse(tp, Errors.UNSUPPORTED_COMPRESSION_TYPE)
Review Comment:
IIUC, we aren't doing on-the-fly down conversion anymore since we've dropped all of Fetch RPC versions that didn't support record version 2.
And for extant data with the older record versions, they are forward compatible with any newer Fetch RPC.
Is that right?
##########
clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java:
##########
@@ -225,33 +214,28 @@ public void clearPartitionRecords() {
data = null;
}
+ //FIXME Check this logic
Review Comment:
Is this still valid?
##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java:
##########
@@ -344,8 +343,7 @@ public RecordAppendResult append(String topic,
}
if (buffer == null) {
- byte maxUsableMagic = apiVersions.maxUsableProduceMagic();
- int size = Math.max(this.batchSize, AbstractRecords.estimateSizeInBytesUpperBound(maxUsableMagic, compression.type(), key, value, headers));
+ int size = Math.max(this.batchSize, AbstractRecords.estimateSizeInBytesUpperBound(RecordBatch.CURRENT_MAGIC_VALUE, compression.type(), key, value, headers));
Review Comment:
nit: can you break this long line?
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1890791622
##########
clients/src/test/java/org/apache/kafka/common/requests/RequestResponseTest.java:
##########
@@ -379,7 +378,7 @@ public void testSerializationSpecialCases() {
checkResponse(createFetchResponse(Errors.FETCH_SESSION_ID_NOT_FOUND, 123), (short) 7);
checkOlderFetchVersions();
// Metadata
- checkRequest(MetadataRequest.Builder.allTopics().build((short) 2));
+ checkRequest(MetadataRequest.Builder.allTopics().build((short) 4)); //FIXME Does version 4 have the special cases?
Review Comment:
I am going through the FIXMEs and will push a commit with the fixes soon.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1892409862
##########
clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java:
##########
@@ -225,33 +214,28 @@ public void clearPartitionRecords() {
data = null;
}
+ //FIXME Check this logic
Review Comment:
This seems fine, I'll just remove the FIXME.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2552207997
> Will we be able to delete LazyDownConversionRecords and the remaining down conversion code after this PR lands?
Yes. This and a few other things are what I meant by:
> Future PRs will remove conditional code that is no longer needed (some of that has been done in KafkaApis,
but only what was required due to the schema changes). We can probably do that in master only as it does
not change behavior.
> I think we'll be leaning on the test suite for this change :)
Yeah, I think that will ensure the remaining protocol versions work correctly. For the clients that are no longer supported, I will test them manually and considering what we can do via the system tests.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1890724339
##########
clients/src/main/resources/common/message/CreateTopicsRequest.json:
##########
@@ -18,6 +18,8 @@
"type": "request",
"listeners": ["zkBroker", "broker", "controller"],
"name": "CreateTopicsRequest",
+ // Versions 0-1 were removed in Apache Kafka 4.0, Version 2 is the new baseline.
Review Comment:
It can be ignored, that is what would have happened if we had used 2.1 as the baseline without the adjustment for third party 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-XXX: Remove deprecated protocol APIs support (KIP-896) [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2546300593
@ijuma just so you know, the JUnit tests won't run on draft PRs.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2569401432
Thanks for checking - it's helpful to make sure!
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1890794358
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -834,81 +832,6 @@ class KafkaApis(val requestChannel: RequestChannel,
}
}
- def maybeConvertFetchedData(tp: TopicIdPartition,
- partitionData: FetchResponseData.PartitionData): FetchResponseData.PartitionData = {
- // We will never return a logConfig when the topic is unresolved and the name is null. This is ok since we won't have any records to convert.
- val logConfig = replicaManager.getLogConfig(tp.topicPartition)
-
- if (logConfig.exists(_.compressionType == BrokerCompressionType.ZSTD) && versionId < 10) {
- trace(s"Fetching messages is disabled for ZStandard compressed partition $tp. Sending unsupported version response to $clientId.")
- FetchResponse.partitionResponse(tp, Errors.UNSUPPORTED_COMPRESSION_TYPE)
Review Comment:
> IIUC, we aren't doing on-the-fly down conversion anymore since we've dropped all of Fetch RPC versions that didn't support record version 2.
Correct.
> And for extant data with the older record versions, they are forward compatible with any newer Fetch RPC.
Correct, KIP-724 explains this in more detail. Unfortunately, the consumers need to support all record versions for now. See "Future Work" for more on what we would have to do to change that.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1890728599
##########
clients/src/main/resources/common/message/FindCoordinatorRequest.json:
##########
@@ -18,6 +18,8 @@
"type": "request",
"listeners": ["zkBroker", "broker"],
"name": "FindCoordinatorRequest",
+ // Versions 0 was removed in Apache Kafka 4.0, Version 1 is the new baseline.
+ //
Review Comment:
Yes, good catch. It looks like this is a second case where we deprecated something incorrectly. I checked the KIP and this was indeed an update:
https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=235838251&selectedPageVersions=18&selectedPageVersions=17
--
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-XXX: Remove deprecated protocol APIs support (KIP-896) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2546434254
This is not yet ready for review, I would like to see if there are any failures before that. But I changed the status so that the tests run.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2569386033
@frankvicky the v0 version is already added back. Please see https://github.com/apache/kafka/commit/2e3b4cb9578d3e1a32478c8e465f361f46c1018c
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1890704140
##########
clients/src/main/resources/common/message/FindCoordinatorRequest.json:
##########
@@ -18,6 +18,8 @@
"type": "request",
"listeners": ["zkBroker", "broker"],
"name": "FindCoordinatorRequest",
+ // Versions 0 was removed in Apache Kafka 4.0, Version 1 is the new baseline.
+ //
Review Comment:
KIP states
> FindCoordinator: none due to clients (Sarama, kafka-python) support – V0-V1 per the baseline
Does this mean we should not be removing any versions?
##########
clients/src/main/resources/common/message/CreateTopicsRequest.json:
##########
@@ -18,6 +18,8 @@
"type": "request",
"listeners": ["zkBroker", "broker", "controller"],
"name": "CreateTopicsRequest",
+ // Versions 0-1 were removed in Apache Kafka 4.0, Version 2 is the new baseline.
Review Comment:
In the KIP, it states
> CreateTopics: V0-V1 due to clients (Sarama) support – V0-V2 per the baseline
Seems we are removing version 0 and 1. What does "V0-V2 per the baseline" mean for this and other RPCs?
##########
clients/src/main/resources/common/message/DescribeConfigsResponse.json:
##########
@@ -17,10 +17,11 @@
"apiKey": 32,
"type": "response",
"name": "DescribeConfigsResponse",
+ // Version 0 was removed in Apache Kafka 4.0, Version 1 is the new baseline.
// Version 1 adds ConfigSource and the synonyms.
// Starting in version 2, on quota violation, brokers send out responses before throttling.
// Version 4 enables flexible versions.
- "validVersions": "0-4",
Review Comment:
Should we remove the `IsDefault` field since it is version 0 only?
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2554788603
@mumrah I merged master to the branch, fixed the FIXMEs (simply removed them) and updated the PR description with the testing I have done so far and what's in progress. I expect the PR is mostly ready, the only thing I intend to do before merging is fixing any system test failures due to this PR. Let me know if you think there's anything else that needs to be done here.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2557634755
Yes @mumrah, that's the plan. And then merge trunk to this PR and then merge this PR once green.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on code in PR #18218:
URL: https://github.com/apache/kafka/pull/18218#discussion_r1890441287
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -834,81 +832,6 @@ class KafkaApis(val requestChannel: RequestChannel,
}
}
- def maybeConvertFetchedData(tp: TopicIdPartition,
- partitionData: FetchResponseData.PartitionData): FetchResponseData.PartitionData = {
- // We will never return a logConfig when the topic is unresolved and the name is null. This is ok since we won't have any records to convert.
- val logConfig = replicaManager.getLogConfig(tp.topicPartition)
-
- if (logConfig.exists(_.compressionType == BrokerCompressionType.ZSTD) && versionId < 10) {
- trace(s"Fetching messages is disabled for ZStandard compressed partition $tp. Sending unsupported version response to $clientId.")
- FetchResponse.partitionResponse(tp, Errors.UNSUPPORTED_COMPRESSION_TYPE)
Review Comment:
I removed this check because it's kind of pointless (this topic config is rarely used and we never checked for the most common case of compression happening on the producer) and added unnecessarily complexity once down conversion was removed. Let me know if this is reasonable.
--
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-18269: Remove deprecated protocol APIs support (KIP-896, KIP-724) [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on PR #18218:
URL: https://github.com/apache/kafka/pull/18218#issuecomment-2569399175
Oh, I should check trunk first.
Thanks for pointing out 🙇🏼
--
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