Skip to content

Conversation

@rangadi
Copy link
Contributor

@rangadi rangadi commented Apr 25, 2016

Fix a flaky KafkaIO test.

KafkaIO reader reads from Kafka in a separate thread. As a result, start() or advance() might not read a record with in 10 millis timeout even from the mock kafka consumer. Updated one test that manually reads from KafkaIO reader to invoke advance() in a loop.

This should fix the flaky test. I am running these tests in a loop on my desktop to verify. One thing I can not explain is the following log from the failed jenkins build linked in BEAM-220 (the log was same when I reproduced it locally):

Apr 22, 2016 7:04:10 AM org.apache.beam.sdk.io.kafka.KafkaIO$UnboundedKafkaReader advance
INFO: Reader-0: first record offset 0

It is entirely clear if this is from the failed test, but first record offset 0 implies the start() must have returned true.

while (!reader.advance()) {
attempts++;
// very rarely will there be more than one attempts.
assertTrue("could not advance() even after 1000 attempts.", attempts < 1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this scenario could still happen if the reader thread gets swapped out but the work thread does not.

Rather just delete this check and let test-level timeouts handle the failure case. These have a reasonable default (20s?) and maybe override it below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. 20s is test timeout is fine. I will remove the check.

@rangadi
Copy link
Contributor Author

rangadi commented Apr 25, 2016

@dhalperi addressed the comments. Please take a look.

@asfgit asfgit closed this in e953cb0 Apr 25, 2016
@dhalperi
Copy link
Contributor

LGTM

@rangadi
Copy link
Contributor Author

rangadi commented Apr 25, 2016

R: @dhalperi (next time :) )

iemejia pushed a commit to iemejia/beam that referenced this pull request Jan 12, 2018
mareksimunek pushed a commit to mareksimunek/beam that referenced this pull request May 9, 2018
[euphoria-core] apache#21 add builder javadocs to operators
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants