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

fix: rebuild connection cache on timeout #2382

Merged

Conversation

Glitchfix
Copy link
Contributor

@Glitchfix Glitchfix commented Dec 7, 2023

What this PR does / why we need it:
When creating the http client we cache the client against the host without any expiration

c = newHTTPClient(u, nil, 10*time.Second, 5*time.Minute)
httpCache[host] = c

In this scenario the node when rebooted was still picking the client from cache for any further request
The cached client had resolved the IP that was assigned to the node prior to reboot hence restarting resolved the issue
A fix for this would be to add a custom round tripper in the transport layer which can invalidate the cache and build a new http client in case of a timeout, this way the it will attempt to resolve the new IP for that host

Which issue(s) this PR fixes (optional)
PWX-35254

Testing Notes
restart one of the storage node and assign it a new IP address without changing the hostname
it will defer the cached client and should build a new client that resolves the correct IP address

@Glitchfix Glitchfix force-pushed the fix/rebuild-connection-cache branch 2 times, most recently from 8df2a9b to bb9d796 Compare December 11, 2023 11:46
@Glitchfix Glitchfix marked this pull request as ready for review December 11, 2023 11:47
Copy link
Contributor

@pnookala-px pnookala-px left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me. Can you add Testing Notes?

Signed-off-by: Shivanjan Chakravorty <[email protected]>
@Glitchfix Glitchfix force-pushed the fix/rebuild-connection-cache branch from ce27973 to 538cc5c Compare December 12, 2023 06:09
@Glitchfix Glitchfix merged commit 14c3328 into libopenstorage:master Dec 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants