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

Permit creating client with multiple broker addresses #13

Merged
merged 1 commit into from
Aug 15, 2013

Conversation

eapache
Copy link
Contributor

@eapache eapache commented Aug 15, 2013

Fixes #7

@burke @fw42

CC @tobi

This does the simplest thing that works. It has some flaws, but most
importantly it gets the API in place and is basically functional. We can
improve it later without breaking API compatibility.

Remaining sub-optimal behaviour:

  • If the user provides three addresses, only the last of which is valid, we
    wait for the first two connections to completely fail (which may take several
    minutes depending on the TCP timeout) before we try the third. Trying them
    all in parallel and using the first valid one would be better.
  • Once we've had an address fail, we discard it forever. Over the lifetime of a
    long-running cluster, all of the nodes may be down at one point or another,
    meaning that eventually we may 'run out' of nodes and abort. We should
    probably just mark the address as recently-failed, and retry them after some
    time has passed. Only if all nodes have recently failed should we abort.

Fixes #7

This does basically the simplest thing that works. It has some flaws, but most
importantly it gets the API in place and is basically functional. We can
improve it later without breaking API compatibility.

Remaining sub-optimal behaviour:
 * If the user provides three addresses, only the last of which is valid, we
   wait for the first two connections to completely fail (which may take several
   minutes depending on the TCP timeout) before we try the third. Trying them
   all in parallel and using the first valid one would be better.
 * Once we've had an address fail, we discard it forever. Over the lifetime of a
   long-running cluster, all of the nodes may be down at one point or another,
   meaning that eventually we may 'run out' of nodes and abort. We should
   probably just mark the address as recently-failed, and retry them after some
   time has passed. Only if all nodes have *recently* failed should we abort.
@tobi
Copy link

tobi commented Aug 15, 2013

Why use the flawed approach? Have a look how db.SQL does it.

-- Tobi
CEO Shopify

(mobile)

On Aug 15, 2013, at 7:00 AM, Evan Huus notifications@github.com wrote:

Fixes #7

@burke @fw42

CC @tobi

This does the simplest thing that works. It has some flaws, but most
importantly it gets the API in place and is basically functional. We can
improve it later without breaking API compatibility.

Remaining sub-optimal behaviour:

If the user provides three addresses, only the last of which is valid, we wait for the first two connections to completely fail (which may take several minutes depending on the TCP timeout) before we try the third. Trying them all in parallel and using the first valid one would be better.
Once we've had an address fail, we discard it forever. Over the lifetime of a long-running cluster, all of the nodes may be down at one point or another, meaning that eventually we may 'run out' of nodes and abort. We should probably just mark the address as recently-failed, and retry them after some time has passed. Only if all nodes have recently failed should we abort.
You can merge this Pull Request by running

git pull https://github.com/Shopify/sarama multiple_broker_client
Or view, comment on, or merge it at:

#13

Commit Summary

Permit creating client with multiple broker addresses
File Changes

M client.go (68)
M client_test.go (17)
M consumer_test.go (4)
M producer_test.go (4)
Patch Links:

https://github.com/Shopify/sarama/pull/13.patch
https://github.com/Shopify/sarama/pull/13.diff

eapache added a commit that referenced this pull request Aug 15, 2013
Permit creating client with multiple broker addresses
@eapache eapache merged commit 693f7e2 into master Aug 15, 2013
@eapache eapache deleted the multiple_broker_client branch August 15, 2013 16:59
faec added a commit to faec/sarama that referenced this pull request Aug 20, 2021
Update Sarama baseline version to 1.27.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initializing Client with Multiple Brokers
2 participants