Kafka
  1. Kafka
  2. KAFKA-902

Randomize backoff on the clients for metadata requests

    Details

    • Type: Bug Bug
    • Status: In Progress
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 0.8.0
    • Fix Version/s: None
    • Component/s: clients
    • Labels:

      Description

      If a Kafka broker dies and there are a large number of clients talking to the Kafka cluster, each of the clients can end up shooting metadata requests at around the same time. It is better to randomize the backoff on the clients so the metadata requests are more evenly spread out

      1. KAFKA-902.patch
        7 kB
        Geoffrey Anderson
      2. KAFKA-902.patch
        25 kB
        Geoffrey Anderson

        Issue Links

          Activity

          Hide
          Jay Kreps added a comment -

          This could be accomplished by just adding +/-5 to the various backoff parameters specified in the producer and consumer.

          However we should still probably interpret 0 as a strict "don't backoff" instruction.

          Show
          Jay Kreps added a comment - This could be accomplished by just adding +/-5 to the various backoff parameters specified in the producer and consumer. However we should still probably interpret 0 as a strict "don't backoff" instruction.
          Hide
          Geoffrey Anderson added a comment -

          Created reviewboard https://reviews.apache.org/r/31633/diff/
          against branch origin/trunk

          Show
          Geoffrey Anderson added a comment - Created reviewboard https://reviews.apache.org/r/31633/diff/ against branch origin/trunk
          Hide
          Grant Henke added a comment -

          A few thoughts on the patch:
          Should the "jitter" be added to 'reconnect.backoff.ms' too?
          Would there ever be a good reason to change the jitter value from 10? Should it be added to the CommonClientConfigs?

          Show
          Grant Henke added a comment - A few thoughts on the patch: Should the "jitter" be added to 'reconnect.backoff.ms' too? Would there ever be a good reason to change the jitter value from 10? Should it be added to the CommonClientConfigs?
          Hide
          Jay Kreps added a comment -

          This looks good to me. I'd second Grant's comments:
          1. I agree we should probably make it configurable and mark the configuration low importance. This kind of configuration is hyper-annoying because no one will ever set it but it's probably the right thing to do.
          2. We should definitely apply the same thing to the reconnect backoff as well as metadata max age (if everyone disconnects at time X they will all expire their metadata at X+metadata.max.age.ms so jittering that will help too).

          Another thing is that this jitter is only additive, so if you configure a backoff of 10 ms, your observed backoff time will be 15 ms. Also 10 ms will be a bit large if you configure a 1 ms backoff and zero ends up being kind of magical. I don't think this is really too terrible and it is simple, so maybe we should just leave it.

          Another possibility would be something like using a jitter that is a random int in +/- min(20, 0.2 * backoff_ms).

          Show
          Jay Kreps added a comment - This looks good to me. I'd second Grant's comments: 1. I agree we should probably make it configurable and mark the configuration low importance. This kind of configuration is hyper-annoying because no one will ever set it but it's probably the right thing to do. 2. We should definitely apply the same thing to the reconnect backoff as well as metadata max age (if everyone disconnects at time X they will all expire their metadata at X+metadata.max.age.ms so jittering that will help too). Another thing is that this jitter is only additive, so if you configure a backoff of 10 ms, your observed backoff time will be 15 ms. Also 10 ms will be a bit large if you configure a 1 ms backoff and zero ends up being kind of magical. I don't think this is really too terrible and it is simple, so maybe we should just leave it. Another possibility would be something like using a jitter that is a random int in +/- min(20, 0.2 * backoff_ms).
          Hide
          Geoffrey Anderson added a comment -

          Thanks for the feedback.

          Is there any reason to keep 0 -> 0 behavior? In other words, is there any harm to making 0 less magical and perturbing it just like any other value?

          Also, I like the idea of scaling the upper bound on jitter with backoff_ms, though it seems like we still want some amount of jitter even if backoff_ms is small (in your example, if backoff_ms < 5, then jitter is always 0)
          In that case, we might want to make sure that the jitter can be non-zero with something like max(3, min(20, 0.2 * backoff_ms))

          But then we end up with a couple more semi-arbitrary parameters - a scaling parameter and a hard minimum.

          Show
          Geoffrey Anderson added a comment - Thanks for the feedback. Is there any reason to keep 0 -> 0 behavior? In other words, is there any harm to making 0 less magical and perturbing it just like any other value? Also, I like the idea of scaling the upper bound on jitter with backoff_ms, though it seems like we still want some amount of jitter even if backoff_ms is small (in your example, if backoff_ms < 5, then jitter is always 0) In that case, we might want to make sure that the jitter can be non-zero with something like max(3, min(20, 0.2 * backoff_ms)) But then we end up with a couple more semi-arbitrary parameters - a scaling parameter and a hard minimum.
          Hide
          Geoffrey Anderson added a comment -

          Created reviewboard https://reviews.apache.org/r/31715/diff/
          against branch origin/trunk

          Show
          Geoffrey Anderson added a comment - Created reviewboard https://reviews.apache.org/r/31715/diff/ against branch origin/trunk
          Hide
          Jun Rao added a comment -

          Thanks for the patch. A few comments.

          1. I feel that it's simpler to express jitter just as a percentage of the backoff. So the actual backoff will be backoff (1 + jitter-percent). This is simpler and it covers the case when backoff = 0 naturally, i.e., if backoff is 0, no jitter is added. It's true that if the backoff is small, no jitter may be added. However, if backoff is really small, adding a small jitter may not spread the retries enough anyway.

          2. We probably need to add jitter as a configuration. Should we add a single jitter config for all backoffs or one per backoff? Probably a single jitter config is enough.

          3. On the broker side, we already have a config for log rolling jittter. It would be good to make the jitter implementation consistent. So, if we pick a new jitter strategy for the clients, we should use the same in the broker as well. This may mean that we will change some of the broker configs. This change will be of low impact, but we probably should file a KIP to keep everyone informed.

          Show
          Jun Rao added a comment - Thanks for the patch. A few comments. 1. I feel that it's simpler to express jitter just as a percentage of the backoff. So the actual backoff will be backoff (1 + jitter-percent). This is simpler and it covers the case when backoff = 0 naturally, i.e., if backoff is 0, no jitter is added. It's true that if the backoff is small, no jitter may be added. However, if backoff is really small, adding a small jitter may not spread the retries enough anyway. 2. We probably need to add jitter as a configuration. Should we add a single jitter config for all backoffs or one per backoff? Probably a single jitter config is enough. 3. On the broker side, we already have a config for log rolling jittter. It would be good to make the jitter implementation consistent. So, if we pick a new jitter strategy for the clients, we should use the same in the broker as well. This may mean that we will change some of the broker configs. This change will be of low impact, but we probably should file a KIP to keep everyone informed.

            People

            • Assignee:
              Geoffrey Anderson
              Reporter:
              Neha Narkhede
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:

                Development