New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KAFKA-4916: test streams with brokers failing #2719
KAFKA-4916: test streams with brokers failing #2719
Conversation
Already getting an error from this basic test, that needs to be fixed before this PR goes in. |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
@dguy want to have a look? Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @enothereska just left a couple of questions
@@ -136,6 +136,9 @@ public void run() { | |||
props.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, kafka); | |||
props.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, ByteArraySerializer.class); | |||
props.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, ByteArraySerializer.class); | |||
props.put(ProducerConfig.RETRIES_CONFIG, 5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should set this higher? is there any harm in setting it to Integer.MAX_VALUE
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
# Pick a random topic and bounce it's leader | ||
topic_index = randint(0, len(self.topics.keys()) - 1) | ||
topic = self.topics.keys()[topic_index] | ||
failures[failure_mode](self, topic, broker_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we should maybe kill some more brokers? Or is that going to be too non-deterministic in terms of test failures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably can. The main problem I have right now is how to sidestep all the Kafka corner cases when it comes to failures while still showing that streams is resilient. Stay tuned.
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
@dguy Making as WiP again since 2 tests are failing and I need to investigate. [2017-03-22 16:25:05,575] ERROR stream-thread [SmokeTest-b25363c8-5bce-4d72-8c51-8fe0b49715f6-StreamThread-1] Streams application error during processing: (org.apache.kafka.streams.processor.internals.StreamThread) |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @enothereska, LGTM
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
retest this please |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Env failure, unrelated to PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): |
Several fixes for handling broker failures: