Skip to content

KAFKA-9798: Send one round synchronously before starting the async producer#8565

Merged
guozhangwang merged 5 commits into
apache:trunkfrom
guozhangwang:K9798-pre-send-one-record
May 4, 2020
Merged

KAFKA-9798: Send one round synchronously before starting the async producer#8565
guozhangwang merged 5 commits into
apache:trunkfrom
guozhangwang:K9798-pre-send-one-record

Conversation

@guozhangwang

Copy link
Copy Markdown
Contributor

Comparing all other test cases, the shouldAllowConcurrentAccesses starts an async producer sending records throughout the test other than just synchronously sent and acked a few records before we start the streams application. Right after the streams app is started, we check that at least one record is sent to the output topic (i.e. completed processing). However since only this test starts the producer async and did not wait for it to complete, it is possible that the async producer gets too longer to produce some records and causing it to fail.

To follow what other tests did, I let this test to first send one round of records synchronously before starting the async producing.

Also encountered some new scala warnings that I fixed along with this PR.

Committer Checklist (excluded from commit message)

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

@guozhangwang

Copy link
Copy Markdown
Contributor Author

@mjsax @cadonna to take a look.

@guozhangwang guozhangwang changed the title KAFKA-9798: Send one record before starting the async producer KAFKA-9798: Send one round synchronously before starting the async producer Apr 28, 2020
@mjsax mjsax added streams tests Test fixes (including flaky tests) labels May 1, 2020
}
incrementIteration();
}
producer.flush();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The producer is create using try-with-resource clause thus, the producer should be flushed on the implicit close() anyway? Thus, I don't think explicit flushing would help?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, will fix.

ProducerRunnable producerRunnable = new ProducerRunnable(streamThree, inputValues, 1);
producerRunnable.run();

producerRunnable = new ProducerRunnable(streamConcurrent, inputValues, numIterations);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be numIterations - 1

@guozhangwang

Copy link
Copy Markdown
Contributor Author

@mjsax thanks for the review! Addressed your comments.

@mjsax mjsax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this will fix the test, but maybe help narrow it down. Feel free to merge.

@guozhangwang
guozhangwang merged commit 34824b7 into apache:trunk May 4, 2020
@guozhangwang
guozhangwang deleted the K9798-pre-send-one-record branch May 4, 2020 01:39
Kvicii pushed a commit to Kvicii/kafka that referenced this pull request May 7, 2020
* 'trunk' of github.com:apache/kafka: (87 commits)
  KAFKA-9865: Expose output topic names from TopologyTestDriver (apache#8483)
  MINOR - Increase the number of Trogdor Histogram buckets to 10000 (apache#8627)
  KAFKA-9768: Fix handling of rest.advertised.listener config (apache#8360)
  KAFKA-9419: Fix possible integer overflow in CircularIterator (apache#7950)
  MINOR: Only add 'Data' suffix for generated request/response/header types (apache#8625)
  KAFKA-9947; Ensure proper shutdown of services in `TransactionsBounceTest` (apache#8602)
  KAFKA-6342; Remove unused workaround for JSON parsing of non-escaped strings (apache#8591)
  MINOR: Pass `-release 8` to scalac and upgrade to Gradle 6.4 (apache#8538)
  KAFKA-9946; Partition deletion event should only be sent if deletion was requested in the StopReplica request (apache#8609)
  MINOR: Improve TopologyTestDriver JavaDocs (apache#8619)
  MINOR: MockAdminClient should return InvalidReplicationFactorException if brokers.size < replicationFactor
  KAFKA-9748: Add Streams eos-beta integration test (apache#8496)
  KAFKA-9731: Disable immediate fetch response for hw propagation if replica selector is not defined (apache#8607)
  HOTFIX: set correct numIterations in shouldAllowConcurrentAccesses
  MINOR: Clean up some test dependencies on ConfigCommand and TopicCommand (apache#8527)
  KAFKA-9918; SslEngineFactory is NOT closed when channel is closing (apache#8551)
  KAFKA-9798: Send one round synchronously before starting the async producer (apache#8565)
  MINOR: Annotate KafkaAdminClientTest.testAlterClientQuotas() with @test
  KAFKA-9589: Enable testLogAppendTimeNonCompressedV2 and fix bug in helper method (apache#8533)
  MINOR: Use min/max function when possible (apache#8577)
  ...

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

Labels

streams tests Test fixes (including flaky tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants