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

Expose host address when timeout exceeded #1943

Closed
wants to merge 4 commits into from

Conversation

laskoviymishka
Copy link
Contributor

Fixes: #1942

Once we face a timeout we should expose address, it can be for example ipv6 or ipv4 and knowing what exact address we took for resolver extreamly helpful for triaging a networking issue.

This issue is also fixed in pgx/v4, see: jackc/pgconn#139

Fixes: jackc#1942

Once we face a timeout we should expose address, it can be for example ipv6 or ipv4 and knowing what exact address we took for resolver extreamly helpful for triaging a networking issue.

This issue is also fixed in pgx/v4, see: jackc/pgconn#139
@jackc
Copy link
Owner

jackc commented Mar 16, 2024

I don't think this is right. errTimeout can some sort of TCP timeout. It is entirely possible that one connection attempt to one host will timeout without the entire connection attempt to all hosts having timed out due to context cancellation or some other global timeout.

@laskoviymishka
Copy link
Contributor Author

It's not possible with current sequential logic, once we receive timeout all other fallback will receive same timeout error

pgconn/pgconn.go Outdated
if _, ok := cerr.err.(*errTimeout); ok {
// once we reach timeout it's useless to check other fallbacks
break
}
Copy link
Owner

Choose a reason for hiding this comment

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

What I'm saying is it's not always useless to check other fallbacks. errTimeout could come from a TCP timeout. If the context has not expired the next fallback should be tried.

Copy link
Contributor Author

@laskoviymishka laskoviymishka Mar 17, 2024

Choose a reason for hiding this comment

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

Got it.
I made one more attempt :D
So now I do a bit different thing, I keep track on fallback error and override result error only if needed.
For a deadline errors we should keep only first deadline error in result, every next one is false-positive errors (since we not actually do anything).

@laskoviymishka
Copy link
Contributor Author

This one is addressed here: 8db9716

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.

Timeout error not show which fallback was used
2 participants