Skip to content

KAFKA-10052: Harden assertion of topic settings in Connect integration tests#8735

Merged
kkonstantine merged 2 commits intoapache:trunkfrom
kkonstantine:kafka-10052
May 28, 2020
Merged

KAFKA-10052: Harden assertion of topic settings in Connect integration tests#8735
kkonstantine merged 2 commits intoapache:trunkfrom
kkonstantine:kafka-10052

Conversation

@kkonstantine
Copy link
Copy Markdown
Contributor

@kkonstantine kkonstantine commented May 27, 2020

A recently added assertion in Connect integration tests uses consumer#partitionsFor to verify that a topic was created with the expected number of partitions and replicas. However, probably because of metadata propagation delays, this call doesn't always return a valid PartitionInfo for the topic that has just been created and the test is terminated with a NPE.

This commit changes the assertion to perform retries in order to verify the topic settings and uses the admin client instead.

Tests have been adjusted to use the new assertion.

Committer Checklist (excluded from commit message)

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

@kkonstantine kkonstantine requested a review from rhauch May 27, 2020 22:02
@kkonstantine
Copy link
Copy Markdown
Contributor Author

ok to test

Copy link
Copy Markdown
Contributor

@rhauch rhauch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @kkonstantine! LGTM, pending a green build.

@rhauch
Copy link
Copy Markdown
Contributor

rhauch commented May 28, 2020

The test failures appear to be unrelated to Connect and are known to be flaky.

Copy link
Copy Markdown
Contributor Author

@kkonstantine kkonstantine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rhauch
Indeed, one green build and no Connect IT failures. I agree that probably it's worth merging. We can return to the tests if we notice that this PR didn't fix flakiness.
Cheers!

@kkonstantine kkonstantine merged commit c89f040 into apache:trunk May 28, 2020
@kkonstantine kkonstantine deleted the kafka-10052 branch May 28, 2020 04:15
Kvicii pushed a commit to Kvicii/kafka that referenced this pull request May 30, 2020
* 'trunk' of github.com:apache/kafka: (36 commits)
  Remove redundant `containsKey` call in KafkaProducer (apache#8761)
  KAFKA-9494; Include additional metadata information in DescribeConfig response (KIP-569) (apache#8723)
  KAFKA-10061; Fix flaky `ReassignPartitionsIntegrationTest.testCancellation` (apache#8749)
  KAFKA-9130; KIP-518 Allow listing consumer groups per state (apache#8238)
  KAFKA-9501: convert between active and standby without closing stores (apache#8248)
  KAFKA-10056; Ensure consumer metadata contains new topics on subscription change (apache#8739)
  MINOR: Log the reason for coordinator discovery failure (apache#8747)
  KAFKA-10029; Don't update completedReceives when channels are closed to avoid ConcurrentModificationException (apache#8705)
  MINOR: remove unnecessary timeout for admin request (apache#8738)
  MINOR: Relax Percentiles test (apache#8748)
  MINOR: regression test for task assignor config (apache#8743)
  MINOR: Update documentation.html to refer to 2.6 (apache#8745)
  MINOR: Update documentation.html to refer to 2.5 (apache#8744)
  KAFKA-9673: Filter and Conditional SMTs (apache#8699)
  KAFKA-9971: Error Reporting in Sink Connectors (KIP-610) (apache#8720)
  KAFKA-10052: Harden assertion of topic settings in Connect integration tests (apache#8735)
  MINOR: Slight MetadataCache tweaks to avoid unnecessary work (apache#8728)
  KAFKA-9802; Increase transaction timeout in system tests to reduce flakiness (apache#8736)
  KAFKA-10050: kafka_log4j_appender.py fixed for JDK11 (apache#8731)
  KAFKA-9146: Add option to force delete active members in StreamsResetter (apache#8589)
  ...

# Conflicts:
#	core/src/main/scala/kafka/log/Log.scala
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants