KAFKA-16692: InvalidRequestException: ADD_PARTITIONS_TO_TXN with version 4 which is not enabled when upgrading from kafka #15971
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jolshan
changed the title
dajac
reviewed
May 16, 2024
Comment on lines
+150
to
+151
| assignment = ":".join(map(str, [self.kafka.idx(node) for node in self.kafka.nodes])) | ||
| transaction_assignment = ",".join(map(str, [assignment[::-1]] * 50)) |
There was a problem hiding this comment.
Let me check if I got this right. You want the leaders of input/output to be on kafka-0 -- the broker with the latest version -- and the transaction state partition to be on the last broker. Is it correct?
There was a problem hiding this comment.
Yes we want to attempt to send a verification request to a broker that might not support it.
| @@ -82,7 +82,7 @@ public static NetworkClient buildNetworkClient(String prefix, | |||
| config.connectionSetupTimeoutMs(), | |||
| config.connectionSetupTimeoutMaxMs(), | |||
| time, | |||
| false, | |||
| true, | |||
There was a problem hiding this comment.
This one is only used by the AddPartitionsToTxnManager, right?
There was a problem hiding this comment.
Yes, I believe this is the only usage now.
jolshan
added a commit
to jolshan/kafka
that referenced
this pull request
May 19, 2024
…ion 4 which is not enabled when upgrading from kafka (apache#15971) We weren't enabling discoverBrokerVersions to check the supported versions in the AddPartitionsToTxnManager. This means that any verification request (or any AddPartitionsToTxnRequest version) from a newer broker would fail when sending to an older broker. The bulk of this change is adding additional transactions system tests for old versions. One test upgrades the cluster completely. This didn't catch the issue but could be useful. The other test forces a new broker to send a verification request to an older one. Without the discoverBrokerVersions change, all tests between mixed brokers failed. (We introduced a new request version in 3.8 -- which is a separate version from the one that caused the bug for 3.5 -> 3.6) With the addition, the tests all passed. I also manually ran a test for 3.5 -> 3.6 since the issue there was slightly different and was caused by the unstableLatestVersion flag being enabled. This change should fix this as well. 👍 Reviewers: David Jacot <[email protected]>
|
I think there was typo on the PR and commit title, it should refer KAFKA-16692. |
jolshan
added a commit
to jolshan/kafka
that referenced
this pull request
May 20, 2024
…ion 4 which is not enabled when upgrading from kafka (apache#15971) We weren't enabling discoverBrokerVersions to check the supported versions in the AddPartitionsToTxnManager. This means that any verification request (or any AddPartitionsToTxnRequest version) from a newer broker would fail when sending to an older broker. The bulk of this change is adding additional transactions system tests for old versions. One test upgrades the cluster completely. This didn't catch the issue but could be useful. The other test forces a new broker to send a verification request to an older one. Without the discoverBrokerVersions change, all tests between mixed brokers failed. (We introduced a new request version in 3.8 -- which is a separate version from the one that caused the bug for 3.5 -> 3.6) With the addition, the tests all passed. I also manually ran a test for 3.5 -> 3.6 since the issue there was slightly different and was caused by the unstableLatestVersion flag being enabled. This change should fix this as well. 👍 Reviewers: David Jacot <[email protected]>
jolshan
added a commit
that referenced
this pull request
May 20, 2024
…ion 4 which is not enabled when upgrading from kafka (#15971) We weren't enabling discoverBrokerVersions to check the supported versions in the AddPartitionsToTxnManager. This means that any verification request (or any AddPartitionsToTxnRequest version) from a newer broker would fail when sending to an older broker. The bulk of this change is adding additional transactions system tests for old versions. One test upgrades the cluster completely. This didn't catch the issue but could be useful. The other test forces a new broker to send a verification request to an older one. Without the discoverBrokerVersions change, all tests between mixed brokers failed. (We introduced a new request version in 3.8 -- which is a separate version from the one that caused the bug for 3.5 -> 3.6) With the addition, the tests all passed. I also manually ran a test for 3.5 -> 3.6 since the issue there was slightly different and was caused by the unstableLatestVersion flag being enabled. This change should fix this as well. 👍 Reviewers: David Jacot <[email protected]>
jolshan
changed the title
|
Yes. I just noticed this too 🤦♀️ Not sure if we want to amend the commit name to fix it. I've picked to 3.6 and 3.7. |
No, that could be very disruptive, not worth the trouble. It wasn't hard to find the JIRA despite the typo.
Nice. Thanks for closing the JIRA too. |
rreddy-22
pushed a commit
to rreddy-22/kafka-rreddy
that referenced
this pull request
May 24, 2024
…ion 4 which is not enabled when upgrading from kafka (apache#15971) We weren't enabling discoverBrokerVersions to check the supported versions in the AddPartitionsToTxnManager. This means that any verification request (or any AddPartitionsToTxnRequest version) from a newer broker would fail when sending to an older broker. The bulk of this change is adding additional transactions system tests for old versions. One test upgrades the cluster completely. This didn't catch the issue but could be useful. The other test forces a new broker to send a verification request to an older one. Without the discoverBrokerVersions change, all tests between mixed brokers failed. (We introduced a new request version in 3.8 -- which is a separate version from the one that caused the bug for 3.5 -> 3.6) With the addition, the tests all passed. I also manually ran a test for 3.5 -> 3.6 since the issue there was slightly different and was caused by the unstableLatestVersion flag being enabled. This change should fix this as well. 👍 Reviewers: David Jacot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We weren't enabling discoverBrokerVersions to check the supported versions in the AddPartitionsToTxnManager. This means that any verification request (or any AddPartitionsToTxnRequest version) from a newer broker would fail when sending to an older broker.
The bulk of this change is adding additional transactions system tests for old versions.
One test upgrades the cluster completely. This didn't catch the issue but could be useful.
The other test forces a new broker to send a verification request to an older one. Without the discoverBrokerVersions change, all tests between mixed brokers failed. (We introduced a new request version in 3.8 -- which is a separate version from the one that caused the bug for 3.5 -> 3.6) With the addition, the tests all passed.
I also manually ran a test for 3.5 -> 3.6 since the issue there was slightly different and was caused by the unstableLatestVersion flag being enabled. This change should fix this as well. 👍
Committer Checklist (excluded from commit message)