KAFKA-2896: Added system test for partition re-assignment #655
Conversation
apovzner
changed the title
|
@granders Please review. |
| "replication-factor": 3, | ||
| 'configs': {"min.insync.replicas": 2}} | ||
| }) | ||
| self.num_partitions = 3 |
There was a problem hiding this comment.
3 seems quite a small number. It might be good to use 20 or something if there isn't a good reason not to.
There was a problem hiding this comment.
I was cautious.... :) good point, I will increase number of partitions.
|
@apovzner - does it pass consistently with bounce enabled? I'm intrigued :) |
|
@benstopford it passed locally few times, and once in Jenkins. I will increase number of partitions and run in Jenkins a few times. |
| self.zk.start() | ||
|
|
||
| def min_cluster_size(self): | ||
| """Override this since we're adding services outside of the constructor""" |
There was a problem hiding this comment.
This is more of a comment than a docstring.
|
@apovzner Test itself looks good to me, just suggested some tweaks to the code and have a question about whether we need all variants. I'm convinced it works reliably -- I see multiple successes on Jenkins and ran it successfully a few times locally. |
|
@apovzner Not much to add after Ben and Ewen's comments. Overall looks good; I agree parse_describe_topic could be refactored elsewhere, and that PLAINTEXT is likely enough. |
…e security protocol for partition re-assignment test
…ribution goals during detection and fixing of goal violations (apache#655)
Reviewers: Manikumar Reddy
Partition re-assignment tests with and without broker failure.