Kafka
  1. Kafka
  2. KAFKA-373

Broker Failure Test needs to be fixed to work with Mirror Maker in TRUNK

    Details

    • Type: Task Task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Embedded consumer is no longer supported and Mirror Maker is introduced to TRUNK. Broker Failure test needs to be fixed to use Mirror Maker for mirroring.

      1. kafka-373-v1.patch
        60 kB
        John Fung
      2. kafka-373-v2.patch
        61 kB
        John Fung

        Activity

        Hide
        John Fung added a comment -

        Uploaded kafka-373-v1.patch. The changes are:

        1. Modified mirror_producer*.properties files to use zk.connect as "broker.list" is no longer supported
        2. The main test script run-test.sh is modified to invoke Mirror Maker for mirroring.
        3. The bouncing option now supports source broker, target broker and mirror maker.
        4. Some methods which can be shared between tests such as "info", "kill_child_processes" are moved to a util.sh script located under "system_test/common"
        5. Updated README

        Show
        John Fung added a comment - Uploaded kafka-373-v1.patch. The changes are: 1. Modified mirror_producer*.properties files to use zk.connect as "broker.list" is no longer supported 2. The main test script run-test.sh is modified to invoke Mirror Maker for mirroring. 3. The bouncing option now supports source broker, target broker and mirror maker. 4. Some methods which can be shared between tests such as "info", "kill_child_processes" are moved to a util.sh script located under "system_test/common" 5. Updated README
        Hide
        Joel Koshy added a comment -

        John, thanks for the patch. Some of these comments would also apply to the
        test in 0.8:

        • The header of the script is mostly in the README which is more
          appropriate. i.e., we should just have a README.
        • Non-blocker: I think it would be good to switch to getopt for the test
          parameters - I think we should do this in our system tests going forward... and some
          of the variable parameters in the script (i.e., the ones you mark as
          "change as needed") can be made command-line parameters.
        • The script is getting to be big, so a number of functions can/should be
          collapsed, and use arguments to handle the variants:
        • wait_for_zero.. functions
        • start_source/target cluster/server functions
        • start_console_consumer functions
        • Shouldn't shutdown_producer do a kill_child_processes? in which case why
          have the file to notify? FWIW, there's some bug in the script's shutdown -
          don't remember what I did, but I had run the system test in 0.8 and my
          hard-disk was full in the morning.
        • Not sure why you need to check the num_iterations ge iter inside the
          start_test loop - also you can move the increment outside those blocks -
          or switch to a for loop over a range.
        • I don't think you need to sleep after starting the mirror makers.
        • Can you elaborate on the hanging console consumer/mirror maker issue?
        Show
        Joel Koshy added a comment - John, thanks for the patch. Some of these comments would also apply to the test in 0.8: The header of the script is mostly in the README which is more appropriate. i.e., we should just have a README. Non-blocker: I think it would be good to switch to getopt for the test parameters - I think we should do this in our system tests going forward... and some of the variable parameters in the script (i.e., the ones you mark as "change as needed") can be made command-line parameters. The script is getting to be big, so a number of functions can/should be collapsed, and use arguments to handle the variants: wait_for_zero.. functions start_source/target cluster/server functions start_console_consumer functions Shouldn't shutdown_producer do a kill_child_processes? in which case why have the file to notify? FWIW, there's some bug in the script's shutdown - don't remember what I did, but I had run the system test in 0.8 and my hard-disk was full in the morning. Not sure why you need to check the num_iterations ge iter inside the start_test loop - also you can move the increment outside those blocks - or switch to a for loop over a range. I don't think you need to sleep after starting the mirror makers. Can you elaborate on the hanging console consumer/mirror maker issue?
        Hide
        John Fung added a comment -

        Hi Joel,

        Thanks for reviewing this patch.

        Uploaded kafka-373-v2.patch and here are the changes:

        1. In "initialize" function, added code to find the location of the zk & kafka log4j log files.

        2. In "cleanup" function, added code to remove the zk & kafka log4j log files

        3. The header of the script is now removed and the description are in README

        4. Use getopt to process command line arguments

        5. Consolidated the following functions:

        • start_console_consumer_for_source_producer
        • start_console_consumer_for_mirror_producer
        • wait_for_zero_source_console_consumer_lags
        • wait_for_zero_mirror_console_consumer_lags

        6. The file to notify producer to stop:
        The producer is sent to the background to run in a while-loop. If a file is used to notify the producer process in the background, the producer will exit properly inside the while loop.

        7. The following check is required for each of the source, target and mirror maker. It is because the following 2 lines are needed for:

        • Line 1: find out if the $bounce_source_id is a char in the string $svr_to_bounce
        • Line 2: check to see if $num_iterations is already reached and if $svr_idx > 0 (meaning this server needs to be bounced)
        • Line 1: svr_idx=`expr index $svr_to_bounce $bounce_source_id`
        • Line 2: if [[ $num_iterations -ge $iter && $svr_idx -gt 0 ]]; then
        Show
        John Fung added a comment - Hi Joel, Thanks for reviewing this patch. Uploaded kafka-373-v2.patch and here are the changes: 1. In "initialize" function, added code to find the location of the zk & kafka log4j log files. 2. In "cleanup" function, added code to remove the zk & kafka log4j log files 3. The header of the script is now removed and the description are in README 4. Use getopt to process command line arguments 5. Consolidated the following functions: start_console_consumer_for_source_producer start_console_consumer_for_mirror_producer wait_for_zero_source_console_consumer_lags wait_for_zero_mirror_console_consumer_lags 6. The file to notify producer to stop: The producer is sent to the background to run in a while-loop. If a file is used to notify the producer process in the background, the producer will exit properly inside the while loop. 7. The following check is required for each of the source, target and mirror maker. It is because the following 2 lines are needed for: Line 1: find out if the $bounce_source_id is a char in the string $svr_to_bounce Line 2: check to see if $num_iterations is already reached and if $svr_idx > 0 (meaning this server needs to be bounced) Line 1: svr_idx=`expr index $svr_to_bounce $bounce_source_id` Line 2: if [[ $num_iterations -ge $iter && $svr_idx -gt 0 ]]; then
        Hide
        Joel Koshy added a comment -

        +1

        The script can be made more concise (basically, a lot more functions can and
        should be moved to utils).

        However, I just realized that since development is primarily on 0.8 and
        because there are a couple of differences in this script between 0.8 and
        trunk, there does not seem to be much point in putting in much effort for
        this in trunk.

        So if there are no objections, I think we can check this in and work on such
        improvements in 0.8 only.

        Show
        Joel Koshy added a comment - +1 The script can be made more concise (basically, a lot more functions can and should be moved to utils). However, I just realized that since development is primarily on 0.8 and because there are a couple of differences in this script between 0.8 and trunk, there does not seem to be much point in putting in much effort for this in trunk. So if there are no objections, I think we can check this in and work on such improvements in 0.8 only.
        Hide
        Joel Koshy added a comment -

        Committed to trunk.

        Show
        Joel Koshy added a comment - Committed to trunk.

          People

          • Assignee:
            John Fung
            Reporter:
            John Fung
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development