MINOR: Adding system test for named repartition topics#5913
Conversation
|
ping @guozhangwang, @mjsax, and @vvcephei for reviews |
There was a problem hiding this comment.
nit: simplify to final String propFileName = args[0]; -- was checked above already
There was a problem hiding this comment.
Should we check if all properties are set (ie, null check)?
There was a problem hiding this comment.
Why do we need this? This is logged anyway.
|
@mjsax, thanks for the review, I've updated per your comments. |
|
Call for second review @guozhangwang @vvcephei |
There was a problem hiding this comment.
would it clarify the pattern matching in the ducktape driver if we make this not a prefix of the other case?
There was a problem hiding this comment.
is it important to have 3 brokers for this test? I'm wondering if the tests would be more resilient and faster with just one broker and replica of each topic.
There was a problem hiding this comment.
not sure that would have an impact, but for this test, a single broker is enough, so I'll adjust it.
There was a problem hiding this comment.
Sorry for the dumb question... I guess this only tails the file, so there's no danger of a false positive from seeing the patter in the file from before the restart?
There was a problem hiding this comment.
Yes that is correct this method delegates to LogMonitor#wait_for which grabs the offset of the file when the object is created and waits for the specified pattern in the file from that point going forward
https://github.com/confluentinc/ducktape/blob/master/ducktape/cluster/remoteaccount.py#L586
https://github.com/confluentinc/ducktape/blob/master/ducktape/cluster/remoteaccount.py#L675
bbejeck
force-pushed
the
MINOR_create_system_test_for_named_repartition_topics
branch
from
95388cb to
91c2b7c
Compare
|
updated this and rebased from trunk |
|
kicked off new system test https://jenkins.confluent.io/job/system-test-kafka-branch-builder/2105/ |
|
Retest this please. |
mjsax
left a comment
mjsax
left a comment
There was a problem hiding this comment.
LGTM -- assuming Jenkins passes
This is a system test for doing a rolling upgrade of a topology with a named repartition topic. 1. An initial Kafka Streams application is started on 3 nodes. The topology has one operation forcing a repartition and the repartition topic is explicitly named. 2. Each node is started and processing of data is validated 3. Then one node is stopped (full stop is verified) 4. A property is set signaling the node to add operations to the topology before the repartition node which forces a renumbering of all operators (except repartition node) 5. Restart the node and confirm processing records 6. Repeat the steps for the other 2 nodes completing the rolling upgrade I ran two runs of the system test with 25 repeats in each run for a total of 50 test runs. All test runs passed Reviewers: John Roesler <john@confluent.io>, Matthias J. Sax <matthias@confluent.io>
bbejeck
deleted the
MINOR_create_system_test_for_named_repartition_topics
branch
4 participants
This is a system test for doing a rolling upgrade of a topology with a named repartition topic.
I ran two runs of the system test with 25 repeats in each run for a total of 50 test runs.
All test runs passed
First 25 test runs
Second 25 test runs
Committer Checklist (excluded from commit message)