[FLINK-13567][e2e] Harden schema registry test #10544
Conversation
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 0c1c909 (Thu Dec 12 10:31:58 UTC 2019) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. DetailsThe Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
GJL
left a comment
GJL
left a comment
There was a problem hiding this comment.
Generally good, I have left 2 questions.
| if ! get_and_verify_schema_subjects_exist; then | ||
| echo "Could not start confluent schema registry" | ||
| exit 1 | ||
| return 1 |
There was a problem hiding this comment.
We are not checking the returned code. Previously, the test would prematurely stop, I think.
There was a problem hiding this comment.
Yes previously it would exit immediately but that makes it impossible to introduce any retry behavior. (well I suppose a sub-shell might work)
The exit code of test_confluent_schema_registry#test_setup is implicitly the return value of #start_confluent_schema_registry, and hence is evaluated by the retry loop.
| } | ||
|
|
||
| function retry_times() { | ||
| retry_times_with_backoff_and_cleanup $1 $2 "${@:3}" "true" |
There was a problem hiding this comment.
How well does it mitigate the issue? Did you run the test with multiple iterations?
|
|
||
| function stop_kafka_cluster { | ||
| $KAFKA_DIR/bin/kafka-server-stop.sh | ||
| if ! [[ -z $(./bin/kafka-server-stop) ]]; then |
There was a problem hiding this comment.
There is missing ".sh" here, which cause FLINK-15428. I'll fix it.
cc @zentol @GJL @tillrohrmann
6 participants
Hardens the schema registry test by retrying the setup of kafka/ZK/registry . For this the existing
retry_timesfunction was extended to optionally include a cleanup command; in this case for shutting down previously started processes.Additionally, if kafka isn't running when shutting it down we now dump the kafka logs to ease debugging.
The last commit in this PR flags the schema test as a pre-commit tests for demonstration purposes.
The exact failure cause of the test is still unknown; what we do know however is that kafka broke down after being successfully started.