You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "brandboat (via GitHub)" <gi...@apache.org> on 2024/12/15 03:27:06 UTC
[PR] [DO NOT MERGE] KAFKA-18206: EmbeddedKafkaCluster must set features [kafka]
brandboat opened a new pull request, #18189:
URL: https://github.com/apache/kafka/pull/18189
### 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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2635438344
> I'm also curious why the tests are returning 1 if there are just flaky tests. Has this always been the case?
Hi @jolshan, pardon me, did you mean EagerConsumerCoordinatorTest.testOutdatedCoordinatorAssignment? If so, yes it it a flaky test, and we already have a JIRA ticket for it: https://issues.apache.org/jira/browse/KAFKA-15900
--
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] [DO NOT MERGE] KAFKA-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2596227623
I'm not sure why, but it seems like a lot of tests have become more flaky than before in this PR. One failed test in CI caught my attention: MirrorConnectorsIntegrationTransactionsTest #testOffsetTranslationBehindReplicationFlow. The error message is:
```
java.lang.RuntimeException: java.util.concurrent.ExecutionException: org.apache.kafka.common.errors.InvalidTxnStateException: The producer attempted a transactional operation in an invalid state.
```
I expected this failed test should to be mitigated after merging [PR #18448](https://github.com/apache/kafka/pull/18448), so I'll spend some time investigating this.
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2625199334
The build is looking better! Maybe we just need to look in to the `KRaftClusterTest` failure?
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2627520283
> The build is looking better! Maybe we just need to look in to the KRaftClusterTest failure?
Yes, I'm currently investigating the root cause of this test failure. Thanks for the heads-up!
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on code in PR #18189:
URL: https://github.com/apache/kafka/pull/18189#discussion_r1942084278
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -696,7 +696,8 @@ class KRaftClusterTest {
new AlterConfigOp(new ConfigEntry("max.connections.per.ip", "60"), OpType.SET))))))
validateConfigs(admin, Map(new ConfigResource(Type.BROKER, "") -> Seq(
("log.roll.ms", "1234567"),
- ("max.connections.per.ip", "60"))), exhaustive = true)
+ ("max.connections.per.ip", "60"),
+ ("min.insync.replicas", "1"))), exhaustive = true)
Review Comment:
after https://github.com/apache/kafka/commit/ec49a60e4f690f311060045ee1feb20d99837f97, if ELR is enabled, a cluster-level min.insync.replicas will be set to 1, and this PR will enable ELR by default in all integration tests that use TestKitNodes.java
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan merged PR #18189:
URL: https://github.com/apache/kafka/pull/18189
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2634829474
I'm also curious why the tests are returning 1 if there are just flaky tests. Has this always been the case?
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2602676101
This https://github.com/apache/kafka/pull/18189#issuecomment-2596227623 seems due to TV_2 return incorrect error, and should be handled correctly in https://github.com/apache/kafka/pull/18604.
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on code in PR #18189:
URL: https://github.com/apache/kafka/pull/18189#discussion_r1942084278
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -696,7 +696,8 @@ class KRaftClusterTest {
new AlterConfigOp(new ConfigEntry("max.connections.per.ip", "60"), OpType.SET))))))
validateConfigs(admin, Map(new ConfigResource(Type.BROKER, "") -> Seq(
("log.roll.ms", "1234567"),
- ("max.connections.per.ip", "60"))), exhaustive = true)
+ ("max.connections.per.ip", "60"),
+ ("min.insync.replicas", "1"))), exhaustive = true)
Review Comment:
after https://github.com/apache/kafka/commit/ec49a60e4f690f311060045ee1feb20d99837f97, if ELR is enabled, a cluster-level min.insync.replicas will be set to 1, and this PR will enable enable ELR by default in all integration tests that use TestKitNodes.java
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on code in PR #18189:
URL: https://github.com/apache/kafka/pull/18189#discussion_r1941600029
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -696,7 +696,8 @@ class KRaftClusterTest {
new AlterConfigOp(new ConfigEntry("max.connections.per.ip", "60"), OpType.SET))))))
validateConfigs(admin, Map(new ConfigResource(Type.BROKER, "") -> Seq(
("log.roll.ms", "1234567"),
- ("max.connections.per.ip", "60"))), exhaustive = true)
+ ("max.connections.per.ip", "60"),
+ ("min.insync.replicas", "1"))), exhaustive = true)
Review Comment:
Why did we make this change?
--
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] [DO NOT MERGE] KAFKA-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2594011900
We have merged 18448 so we should be able to proceed 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] [DO NOT MERGE] KAFKA-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2585045515
> note that this PR also fix potential transaction with empty records in sendPrivileged method as transaction version 2 doesn't allow this kind of scenario.
Should we split this into it's own PR?
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2614845790
Bunch of ci failed in this PR since ClearElrRecord isn't corrected handled, but will be fixed after https://github.com/apache/kafka/pull/18708 merged.
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2629444595
Finally got an AC 🎉 , perhaps we can move 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] [DO NOT MERGE] KAFKA-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2585057503
> Should we split this into it's own PR?
Hi @mjsax, sorry I forgot to mention that this was handling in a different issue, here is the PR: https://github.com/apache/kafka/pull/18448
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "brandboat (via GitHub)" <gi...@apache.org>.
brandboat commented on code in PR #18189:
URL: https://github.com/apache/kafka/pull/18189#discussion_r1942084278
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -696,7 +696,8 @@ class KRaftClusterTest {
new AlterConfigOp(new ConfigEntry("max.connections.per.ip", "60"), OpType.SET))))))
validateConfigs(admin, Map(new ConfigResource(Type.BROKER, "") -> Seq(
("log.roll.ms", "1234567"),
- ("max.connections.per.ip", "60"))), exhaustive = true)
+ ("max.connections.per.ip", "60"),
+ ("min.insync.replicas", "1"))), exhaustive = true)
Review Comment:
after https://github.com/brandboat/kafka/commit/ec49a60e4f690f311060045ee1feb20d99837f97, if ELR is enabled, a cluster-level min.insync.replicas will be set to 1, and this PR will enable enable ELR by default in all integration tests that use TestKitNodes.java
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2637555429
Also pushed to 4.0. Thanks!
--
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-18206: EmbeddedKafkaCluster must set features [kafka]
Posted by "jolshan (via GitHub)" <gi...@apache.org>.
jolshan commented on PR #18189:
URL: https://github.com/apache/kafka/pull/18189#issuecomment-2608342659
@brandboat I just merged my PR and will restart the tests 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