[jira] [Commented] (KAFKA-306) broker failure system test broken on replication branch

["Joel Koshy (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KAFKA-306?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13406230#comment-13406230 ] 

Joel Koshy commented on KAFKA-306:
----------------------------------

John, thanks for the patch. The test script itself looks good - as we
discussed on the other jira we can do further cleanup separately. Here are
some comments on the new changes:

ProducerPerformance:
- seqIdStartFromopt -> startId or initialId would be more
  convenient/intuitive.
- May be better not to describe the message format in detail in the help
  message. I think the template: "Message:000..1:xxx..." is good enough.
- On line 136, 137 I think you mean if (options.has) and not
  if(!options.has) - something odd there. Can you double-check?
- Try to avoid using vars if possible. vals are generally clearer and safer
  - for example,
  val isFixSize = options.has(seqIdStartFromOpt) || !options.has(varyMessageSizeOpt)
  val numThreads = if (options.has(seqIdStartFromOpt)) 1 else options.valueOf(numThreadsOpt).intValue()
  etc.
- For user-specified options that you override can you log a warning?
- Instead of the complicated padding logic I think you can get it for free
  with Java format strings - i.e., specify the width/justification of each
  column in the format string. That would be much easier I think.
- numThreads override to 1 -> did it work to prefix the id with thread-id
  and allow > 1 thread?

Server property files:
- send/receive.buffer.size don't seem to be valid config options - may be
  deprecated by the socket buffer size settings, but not sure.

Util functions:

- Small suggestion: would be better to echo the result than return. So you
  can have: idx=$(get_random_range ...) which is clearer than
  get_random_range; idx=$? . Also, non-zero bash returns typically indicate
  an error.

                
> broker failure system test broken on replication branch
> -------------------------------------------------------
>
>                 Key: KAFKA-306
>                 URL: https://issues.apache.org/jira/browse/KAFKA-306
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.8
>            Reporter: Neha Narkhede
>            Assignee: John Fung
>              Labels: replication
>         Attachments: kafka-306-v1.patch, kafka-306-v2.patch, kafka-306-v3.patch, kafka-306-v4.patch, kafka-306-v5.patch, kafka-306-v6.patch
>
>
> The system test in system_test/broker_failure is broken on the replication branch. This test is a pretty useful failure injection test that exercises the consumer rebalancing feature, various replication features like leader election. It will be good to have this test fixed as well as run on every checkin to the replication branch

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira