MINOR: Partition is under reassignment when adding and removing #8364

jsancio · 2020-03-26T15:28:15Z

A partition is under reassignment if the either the set of adding
replicas or set removing replicas is non-empty.

Fix the test assertion such that it prints stdout on failure.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

core/src/test/scala/unit/kafka/admin/TopicCommandWithAdminClientTest.scala

core/src/main/scala/kafka/admin/TopicCommand.scala

ijuma · 2020-03-31T03:09:06Z

ok to test

jsancio · 2020-03-31T16:19:00Z

@ijuma, we got the following error. It doesn't look related to this change

org.apache.kafka.streams.integration.QueryableStateIntegrationTest.concurrentAccesses failed, log available in /home/jenkins/jenkins-slave/workspace/kafka-pr-jdk11-scala2.13/streams/build/reports/testOutput/org.apache.kafka.streams.integration.QueryableStateIntegrationTest.concurrentAccesses.test.stdout

org.apache.kafka.streams.integration.QueryableStateIntegrationTest > concurrentAccesses FAILED
    java.lang.AssertionError: Did not receive all 48 records from topic output-concurrent-windowed-2 within 120000 ms
    Expected: is a value equal to or greater than <48>
         but: <0> was less than <48>
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
        at org.apache.kafka.streams.integration.utils.IntegrationTestUtils.lambda$waitUntilMinValuesRecordsReceived$6(IntegrationTestUtils.java:691)
        at org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:415)
        at org.apache.kafka.test.TestUtils.retryOnExceptionWithTimeout(TestUtils.java:383)
        at org.apache.kafka.streams.integration.utils.IntegrationTestUtils.waitUntilMinValuesRecordsReceived(IntegrationTestUtils.java:687)
        at org.apache.kafka.streams.integration.QueryableStateIntegrationTest.waitUntilAtLeastNumRecordProcessed(QueryableStateIntegrationTest.java:1199)
        at org.apache.kafka.streams.integration.QueryableStateIntegrationTest.concurrentAccesses(QueryableStateIntegrationTest.java:650)

ijuma · 2020-03-31T16:33:19Z

So it turns out that this was an actual bug. Could we add a unit test that fails without this change?

core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala

jsancio · 2020-03-31T21:49:37Z

core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala

+
+final class TopicCommandTest { 
+  @Test
+  def testIsNotUnderReplicatedWhenAdding(): Unit = {


This test fails against trunk

kafka.admin.TopicCommandTest > testIsNotUnderReplicatedWhenAdding FAILED java.lang.AssertionError at org.junit.Assert.fail(Assert.java:87) at org.junit.Assert.assertTrue(Assert.java:42) at org.junit.Assert.assertFalse(Assert.java:65) at org.junit.Assert.assertFalse(Assert.java:75) at kafka.admin.TopicCommandTest.testIsNotUnderReplicatedWhenAdding(TopicCommandTest.scala:54) 1 test completed, 1 failed

jsancio · 2020-03-31T21:51:49Z

So it turns out that this was an actual bug. Could we add a unit test that fails without this change?

@ijuma , Done.

Renamed TopicCommandTest to TopicCommandWithZKClientTest
Created a suite TopicCommandTest and added a unit test for this change. The new test fails against trunk.

ijuma · 2020-03-31T22:21:34Z

ok to test

ijuma · 2020-03-31T22:22:38Z

Hmm, maybe we should add the new test suite in a separate PR to avoid the large diff (rename detection would then kick in). What do you think?

A partition is under reassignment if the either the set of adding replicas or set removing replicas is non-empty. Fix the test assertion such that it prints stdout on failure.

core/src/test/scala/unit/kafka/admin/TopicCommandWithZKClientTest.scala

jsancio · 2020-04-01T20:02:47Z

@ijuma PR ready.

ijuma · 2020-04-01T20:11:06Z

Ok to test

ijuma · 2020-04-23T14:32:49Z

retest this please

core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala

…ated-test

jsancio · 2020-04-24T22:05:47Z

Thanks for the review @ijuma

ijuma · 2020-04-24T22:18:19Z

ok to test

ijuma

LGTM, thanks!

ijuma · 2020-04-24T22:20:43Z

retest this please

ijuma · 2020-04-24T23:18:39Z

ok to test

ijuma · 2020-04-25T20:53:28Z

JDK 8 job failed due to infra issue, the other 2 passed.

ijuma · 2020-04-25T20:53:34Z

retest this please

ijuma · 2020-04-26T02:31:21Z

Unrelated failures for JDK 8:

kafka.api.CustomQuotaCallbackTest.testCustomQuotaCallback
kafka.api.CustomQuotaCallbackTest.testCustomQuotaCallback
kafka.api.PlaintextConsumerTest.testLowMaxFetchSizeForRequestAndPartition
kafka.api.PlaintextConsumerTest.testLowMaxFetchSizeForRequestAndPartition

…t-for-generated-requests * apache-github/trunk: (366 commits) MINOR: Improve producer test BufferPoolTest#testCloseNotifyWaiters. (apache#7982) MINOR: document how to escape json parameters to ducktape tests (apache#8546) KAFKA-9885; Evict last members of a group when the maximum allowed is reached (apache#8525) KAFKA-9866: Avoid election for topics where preferred leader is not in ISR (apache#8524) KAFKA-9839; Broker should accept control requests with newer broker epoch (apache#8509) KAKFA-9612: Add an option to kafka-configs.sh to add configs from a prop file (KIP-574) MINOR: Partition is under reassignment when adding and removing (apache#8364) MINOR: reduce allocations in log start and recovery checkpoints (apache#8467) MINOR: Remove unused foreign-key join class (apache#8547) HOTFIX: Fix broker bounce system tests (apache#8532) KAFKA-9704: Fix the issue z/OS won't let us resize file when mmap. (apache#8224) KAFKA-8639: Replace AddPartitionsToTxn with Automated Protocol (apache#8326) MINOR: equals() should compare all fields for generated classes (apache#8539) KAFKA-9844; Fix race condition which allows more than maximum number of members(apache#8454) KAFKA-9823: Remember the sent generation for the coordinator request (apache#8445) KAFKA-9883: Add better error message when REST API forwards a request and leader is not known (apache#8536) KAFKA-9907: Switch default build to Scala 2.13 (apache#8537) MINOR: Some html fixes in Streams DSL documentation (apache#8503) MINOR: Enable fatal warnings with scala 2.13 (apache#8429) KAFKA-9852: Change the max duration that calls to the buffer pool can block from 2000ms to 10ms to reduce overall test runtime (apache#8464) ...

ijuma reviewed Mar 27, 2020

View reviewed changes

core/src/test/scala/unit/kafka/admin/TopicCommandWithAdminClientTest.scala Outdated Show resolved Hide resolved

jsancio force-pushed the minor-under-replicated-test branch from 32d2047 to 98e7255 Compare March 30, 2020 20:46

jsancio changed the title ~~MINOR: Assume that metadata replies are eventually consistent.~~ MINOR: Parition is under reassignment when adding and removing Mar 30, 2020

ijuma reviewed Mar 30, 2020

View reviewed changes

core/src/main/scala/kafka/admin/TopicCommand.scala Outdated Show resolved Hide resolved

ijuma changed the title ~~MINOR: Parition is under reassignment when adding and removing~~ MINOR: Partition is under reassignment when adding and removing Mar 31, 2020

jsancio commented Mar 31, 2020

View reviewed changes

core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala Show resolved Hide resolved

jsancio commented Mar 31, 2020

View reviewed changes

jsancio mentioned this pull request Mar 31, 2020

MINOR: Rename TopicCommandTest #8398

Merged

3 tasks

jsancio added 3 commits April 1, 2020 12:51

MINOR: Parition is under reassignment when adding and removing

eb0bc80

A partition is under reassignment if the either the set of adding replicas or set removing replicas is non-empty. Fix the test assertion such that it prints stdout on failure.

Use intValue instead of unbox

a7d69ce

Unit test for false URP when adding

f276d28

jsancio force-pushed the minor-under-replicated-test branch from 2a21d44 to f276d28 Compare April 1, 2020 19:59

jsancio commented Apr 1, 2020

View reviewed changes

core/src/test/scala/unit/kafka/admin/TopicCommandWithZKClientTest.scala Outdated Show resolved Hide resolved

ijuma reviewed Apr 24, 2020

View reviewed changes

core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/trunk' into minor-under-replic…

82afacc

…ated-test

jsancio force-pushed the minor-under-replicated-test branch from ebc391a to 0030c75 Compare April 24, 2020 22:01

Use CollectionConverters

e4e2760

jsancio force-pushed the minor-under-replicated-test branch from 0030c75 to e4e2760 Compare April 24, 2020 22:04

ijuma approved these changes Apr 24, 2020

View reviewed changes

ijuma merged commit d63eaaa into apache:trunk Apr 26, 2020

jsancio deleted the minor-under-replicated-test branch July 14, 2021 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MINOR: Partition is under reassignment when adding and removing #8364

MINOR: Partition is under reassignment when adding and removing #8364

jsancio commented Mar 26, 2020 •

edited

ijuma commented Mar 31, 2020

jsancio commented Mar 31, 2020

ijuma commented Mar 31, 2020

jsancio Mar 31, 2020

jsancio commented Mar 31, 2020

ijuma commented Mar 31, 2020

ijuma commented Mar 31, 2020

jsancio commented Apr 1, 2020

ijuma commented Apr 1, 2020

ijuma commented Apr 23, 2020

jsancio commented Apr 24, 2020

ijuma commented Apr 24, 2020

ijuma left a comment

ijuma commented Apr 24, 2020

ijuma commented Apr 24, 2020

ijuma commented Apr 25, 2020

ijuma commented Apr 25, 2020

ijuma commented Apr 26, 2020

MINOR: Partition is under reassignment when adding and removing #8364

MINOR: Partition is under reassignment when adding and removing #8364

Conversation

jsancio commented Mar 26, 2020 • edited

Committer Checklist (excluded from commit message)

ijuma commented Mar 31, 2020

jsancio commented Mar 31, 2020

ijuma commented Mar 31, 2020

jsancio Mar 31, 2020

Choose a reason for hiding this comment

jsancio commented Mar 31, 2020

ijuma commented Mar 31, 2020

ijuma commented Mar 31, 2020

jsancio commented Apr 1, 2020

ijuma commented Apr 1, 2020

ijuma commented Apr 23, 2020

jsancio commented Apr 24, 2020

ijuma commented Apr 24, 2020

ijuma left a comment

Choose a reason for hiding this comment

ijuma commented Apr 24, 2020

ijuma commented Apr 24, 2020

ijuma commented Apr 25, 2020

ijuma commented Apr 25, 2020

ijuma commented Apr 26, 2020

jsancio commented Mar 26, 2020 •

edited