You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/05/01 02:39:29 UTC
[GitHub] [kafka] dengziming opened a new pull request, #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
dengziming opened a new pull request, #12112:
URL: https://github.com/apache/kafka/pull/12112
*More detailed description of your change*
Currently, we are waiting for metadataCache to bookkeeper the partition info, this isn't enough, we should wait until the partition ISR is less than AR.
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### 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
[GitHub] [kafka] showuon merged pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
showuon merged PR #12112:
URL: https://github.com/apache/kafka/pull/12112
--
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
[GitHub] [kafka] showuon commented on pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
showuon commented on PR #12112:
URL: https://github.com/apache/kafka/pull/12112#issuecomment-1114161084
@dengziming , thanks for the PR. But I'm not sure which issue you're trying to resolve here, because I didn't see the test failed recently. Is it [KAFKA-9341](https://issues.apache.org/jira/browse/KAFKA-9341)? 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
[GitHub] [kafka] showuon commented on pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
showuon commented on PR #12112:
URL: https://github.com/apache/kafka/pull/12112#issuecomment-1123139779
@dengziming , thanks for the update!
@divijvaidya , if you have no other comments, I'm going to merge this PR within this week.
Thank you.
--
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
[GitHub] [kafka] dengziming commented on pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
dengziming commented on PR #12112:
URL: https://github.com/apache/kafka/pull/12112#issuecomment-1123114260
Hello @showuon @divijvaidya, #12108 has been merged, we added `TestUtils.ensureConsistentKRaftMetadata` to fix all this type of flakiness, PTAL.
--
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
[GitHub] [kafka] divijvaidya commented on a diff in pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
divijvaidya commented on code in PR #12112:
URL: https://github.com/apache/kafka/pull/12112#discussion_r862714992
##########
core/src/test/scala/integration/kafka/admin/TopicCommandIntegrationTest.scala:
##########
@@ -586,11 +586,14 @@ class TopicCommandIntegrationTest extends KafkaServerTestHarness with Logging wi
try {
killBroker(0)
val aliveServers = brokers.filterNot(_.config.brokerId == 0)
- TestUtils.waitForPartitionMetadata(aliveServers, testTopicName, 0)
+ TestUtils.waitUntilTrue(
Review Comment:
Can we alternatively use one of the existing methods in TestUtils to validate that the topic partition ISR contains rest of the 5 brokers e.g.
using `TestUtils.waitForBrokersInIsr` could validate that the topic partition metadata exists in expected number of Isr even after one of the brokers is terminated.
--
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
[GitHub] [kafka] dengziming commented on pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
dengziming commented on PR #12112:
URL: https://github.com/apache/kafka/pull/12112#issuecomment-1118110191
@showuon I seem to see it failed but I can't find it anymore, I will close this after I can't see it fail again in a few days.
@divijvaidya I would fix your comment if this test really fails again.
--
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
[GitHub] [kafka] dengziming commented on a diff in pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
dengziming commented on code in PR #12112:
URL: https://github.com/apache/kafka/pull/12112#discussion_r866494535
##########
core/src/test/scala/integration/kafka/admin/TopicCommandIntegrationTest.scala:
##########
@@ -586,11 +586,14 @@ class TopicCommandIntegrationTest extends KafkaServerTestHarness with Logging wi
try {
killBroker(0)
val aliveServers = brokers.filterNot(_.config.brokerId == 0)
- TestUtils.waitForPartitionMetadata(aliveServers, testTopicName, 0)
+ TestUtils.waitUntilTrue(
Review Comment:
@divijvaidya I really appreciate your suggestion, however, I need to clarify something here:
1. We should use `TestUtils.waitForBrokersOutOfIsr` not `TestUtils.waitForBrokersInIsr`
2. `TestUtils.waitForBrokersInIsr` needs to send RPC but we can directly use MetadataCache which is in the memory, so it's faster and more reliable, maybe we can improve `TestUtils.waitForBrokersInIsr` in the future
3. This is most important, in another PR we found we can use a more generic way in KRaft mode, which waits until the brokers have caught up to the controller metadata topic end offset, instead of waiting for something really specific.
--
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
[GitHub] [kafka] dengziming commented on pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
dengziming commented on PR #12112:
URL: https://github.com/apache/kafka/pull/12112#issuecomment-1119266873
@showuon I have updated it, though @divijvaidya provided a good suggestion, we still have a more generic solution for this kind of flakiness. [see details here](https://github.com/apache/kafka/pull/12108#discussion_r865373299)
--
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
[GitHub] [kafka] showuon commented on pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
showuon commented on PR #12112:
URL: https://github.com/apache/kafka/pull/12112#issuecomment-1119226457
@dengziming , it's fine, we can improve the tests anyway. I agree your fix is better. Let's address @divijvaidya 's comment. 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
[GitHub] [kafka] dengziming commented on pull request #12112: MINOR: Fix flaky testDescribeUnderReplicatedPartitions
Posted by GitBox <gi...@apache.org>.
dengziming commented on PR #12112:
URL: https://github.com/apache/kafka/pull/12112#issuecomment-1120105686
Let's hold on to this before [#12108](https://github.com/apache/kafka/pull/12108) was approved.
--
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