Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-2435: Fair consumer partition assignment strategy #146

Closed
wants to merge 1 commit into from

Conversation

noslowerdna
Copy link
Contributor

No description provided.

@asfbot
Copy link

asfbot commented Aug 18, 2015

kafka-trunk-git-pr #158 FAILURE
Looks like there's a problem with this pull request

@asfbot
Copy link

asfbot commented Aug 19, 2015

kafka-trunk-git-pr #170 FAILURE
Looks like there's a problem with this pull request

@noslowerdna
Copy link
Contributor Author

I don't believe that the @asfbot test failure is at all related to this change. I've built the project successfully locally several times.

kafka.api.QuotasTest > testThrottledProducerConsumer FAILED
    junit.framework.AssertionFailedError: Should have been throttled
        at junit.framework.Assert.fail(Assert.java:47)
        at junit.framework.Assert.assertTrue(Assert.java:20)
        at kafka.api.QuotasTest.testThrottledProducerConsumer(QuotasTest.scala:136)

@noslowerdna
Copy link
Contributor Author

Looks like #160 may address this failed test...

@hachikuji
Copy link
Contributor

@noslowerdna Out of curiosity, do you have any use cases where the consumers are subscribing to different sets of topics? The only such case I can think of is during a rolling upgrade.

@noslowerdna
Copy link
Contributor Author

@hachikuji Our primary use case is similar to a rolling upgrade - the members of a consumer group independently retrieve configuration on a regular interval about what topics to subscribe to, and close/refresh their consumer stream whenever the configured set of topics changes. A config change should eventually propagate to all consumers within a few minutes, but we want to still be processing optimally and keep the number of fetcher threads as evenly balanced as possible during the transition window.

@hachikuji
Copy link
Contributor

@noslowerdna Since the consumer supports differing subscriptions, it may make sense for us to include an assignor which takes that into account (even if there don't seem to be any use cases which call for differing subscriptions in a steady state). That said, this change does impact the client configuration (even if in a compatible way), so I wonder if we need a KIP. Also, the new consumer allows users to provide their own assignor implementation, so if the use case is not too common, it may be better to leave it out of core Kafka. What do you think?

@noslowerdna
Copy link
Contributor Author

@hachikuji I will go ahead and create the KIP documentation for this. It's a minor change but still worth discussing with a wider audience.

I implemented an equivalent "fair" assignor for the new consumer last night (#979). For that pull request I added a new test (testMultipleConsumersUnbalancedSubscriptions) for each of the 3 implementations, that illustrates the skewed assignments for the range and roundrobin assignors with a borderline worst-case scenario.

For the new consumer we could certainly include a custom implementation in our own code if the KIP gets rejected. In that case I think we should at least document somewhere that the roundrobin assignor can give somewhat suboptimal results if the subscriptions are not identical (which is/will be allowed as of https://issues.apache.org/jira/browse/KAFKA-2172).

@noslowerdna
Copy link
Contributor Author

@hachikuji It seems that I do not have sufficient permissions to create the KIP wiki page. Here's an initial draft: https://gist.github.com/noslowerdna/577e25182d69306532d1

@hachikuji
Copy link
Contributor

@noslowerdna What's your apache id? I can ask someone to give you access.

@noslowerdna
Copy link
Contributor Author

@hachikuji Thanks, I'm "noslowerdna" there also.

@hachikuji
Copy link
Contributor

@noslowerdna I asked @gwenshap to give you wiki permissions.

@noslowerdna
Copy link
Contributor Author

@asfbot
Copy link

asfbot commented Jan 3, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/442/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link

asfbot commented Jan 3, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/443/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link

asfbot commented Jan 3, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/441/
Test PASSed (JDK 7 and Scala 2.10).

apurvam pushed a commit to apurvam/kafka that referenced this pull request Mar 24, 2017
* add all broker-side configs
* check for transaction timeout value
* added one more exception type
@ghost
Copy link

ghost commented Mar 24, 2017

Is this change set still active?

@asfbot
Copy link

asfbot commented Mar 25, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/2373/
Test PASSed (JDK 7 and Scala 2.10).

@asfbot
Copy link

asfbot commented Mar 25, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/2373/
Test FAILed (JDK 8 and Scala 2.12).

@asfbot
Copy link

asfbot commented Mar 25, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/2377/
Test FAILed (JDK 8 and Scala 2.11).

hachikuji pushed a commit to hachikuji/kafka that referenced this pull request Mar 30, 2017
* add all broker-side configs
* check for transaction timeout value
* added one more exception type
apurvam pushed a commit to apurvam/kafka that referenced this pull request Apr 3, 2017
* add all broker-side configs
* check for transaction timeout value
* added one more exception type
dguy pushed a commit to dguy/kafka that referenced this pull request Apr 13, 2017
* add all broker-side configs
* check for transaction timeout value
* added one more exception type
@hachikuji
Copy link
Contributor

Closing this since the JIRA has been resolved as "won't fix."

@hachikuji hachikuji closed this Feb 25, 2018
ambroff pushed a commit to ambroff/kafka that referenced this pull request May 7, 2021
…ther to turn on the controller request merging feature (apache#146)

TICKET = N/A

LI_DESCRIPTION =
This PR adds a new config li.combined.control.request.enable and the znode
/li_combined_control_request_flag to control whether the controller request merging feature should
be turned on or off.

EXIT_CRITERIA = N/A
wyuka pushed a commit to wyuka/kafka that referenced this pull request Mar 4, 2022
…ther to turn on the controller request merging feature (apache#146)

TICKET = N/A

LI_DESCRIPTION =
This PR adds a new config li.combined.control.request.enable and the znode
/li_combined_control_request_flag to control whether the controller request merging feature should
be turned on or off.

EXIT_CRITERIA = N/A
wyuka pushed a commit to wyuka/kafka that referenced this pull request Mar 28, 2022
…ther to turn on the controller request merging feature (apache#146)

TICKET = N/A

LI_DESCRIPTION =
This PR adds a new config li.combined.control.request.enable and the znode
/li_combined_control_request_flag to control whether the controller request merging feature should
be turned on or off.

EXIT_CRITERIA = N/A
wyuka pushed a commit to wyuka/kafka that referenced this pull request Jun 16, 2022
…ther to turn on the controller request merging feature (apache#146)

TICKET = N/A

LI_DESCRIPTION =
This PR adds a new config li.combined.control.request.enable and the znode
/li_combined_control_request_flag to control whether the controller request merging feature should
be turned on or off.

EXIT_CRITERIA = N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants