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

refresh_slots fix dns resolve delay #61

Merged
merged 1 commit into from
Oct 29, 2023

Conversation

alexander-shabanov
Copy link

Temp fix: Change 'addresses to connections' (specifically get_socket_addrs) to be sequential instead of concurrent in order to workaround DNS query throttling

@nihohit
Copy link

nihohit commented Oct 25, 2023

why does DNS query throttling make it slower than sequential attempts?

@barshaul
Copy link

barshaul commented Oct 26, 2023

@nihohit We haven't dived deep into why it takes longer - it look like it's due to DNS throttling.
ATM we see that it takes 5 seconds when we run it concurrently VS ~500 milliseconds when we run it sequentially, in a large cluster test with 300 nodes.
We should RC it but at the moment lets have this fix and work to solve it later.
@alexander-shabanov have you opened an issue for this?

@barshaul
Copy link

@alexander-shabanov Can you test this change with a small cluster - lets say 3 shards, master&replica in each, to see what's the impact in terms of time to create the client?

@alexander-shabanov
Copy link
Author

@nihohit @barshaul I have submitted an issue make DNS resolution and creation of connections to be concurrent to follow up this fix.

@alexander-shabanov
Copy link
Author

@barshaul For small cluster (3 shards, 9 nodes) sequential DNS resolve takes 15 milliseconds. Client creation overall takes around 100 milliseconds.

@barshaul
Copy link

@alexander-shabanov Please pull latest changes

@alexander-shabanov
Copy link
Author

alexander-shabanov commented Oct 29, 2023

@barshaul I pulled latest changes

@barshaul barshaul merged commit db09996 into amazon-contributing:main Oct 29, 2023
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.

3 participants