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

Fail Fast for Certain Errors #147

Open
mariotaku opened this issue Mar 12, 2024 · 9 comments
Open

Fail Fast for Certain Errors #147

mariotaku opened this issue Mar 12, 2024 · 9 comments

Comments

@mariotaku
Copy link

Hi! I implemented a SSH connection pool in my project, and noticed when the connection can't be created (no route to host, failed to authenticate etc), get_timeout will actually retry until timeout.

I'd like to have an option to allow fail-fast for such case. In my fork, I added can_retry function in HandleError, and when it returns false, it will fail immediately without retry.

6ab3302...aaa2a2b

Do you think this feature makes sense? If so, I'll create a pull request.

@guilhermedelyra
Copy link

@sfackler would you mind taking a look into this? i would greatly appreciate this feature...

sfu-db/connector-x#710

@mariotaku
Copy link
Author

mariotaku commented Nov 22, 2024

@guilhermedelyra I have tested that feature for a while and it worked pretty fine. In the meantime you could use patch in Cargo.toml to use my commit ID for now.

@guilhermedelyra
Copy link

guilhermedelyra commented Nov 23, 2024

@guilhermedelyra I have tested that feature for a while and it worked pretty fine. In the meantime you could use patch in Cargo.toml to use my commit ID for now.

thanks for the suggestion! although i'm not sure if i'll be able to do this...

i'm using python's Connectorx, that relies on rust's Connectorx implementation, which has a wrapper on top of r2d2 😅

since i'm not building connectorx from scratch (i'm using pip install connectorx), i dont think i'm able to patch the lib hehe

but again, thanks for starting the discussion and opening the PR 😀

@pangjunrong
Copy link

Hey @sfackler, bumping this issue as the retry logic currently performs 7 attempts over 30 seconds. When coupled with a wrong password in an enterprise setting, MSSQL accounts get locked as it exceed the number of attempts allowed by DBA. Hence, having the option to fail fast can prevent this and allow for immediate remediation.

@sfackler
Copy link
Owner

Do other connection pool libraries have this concept? What kinds of errors other than auth issues would be considered non-retryable?

@mariotaku
Copy link
Author

@sfackler SQLAlchemy has something similar: https://docs.sqlalchemy.org/en/20/core/pooling.html#supporting-new-database-error-codes-for-disconnect-scenarios

As for the scenarios, I'm using r2d2 for maintaining SSH connections to devices in local network. When connection refused error occurs, it considers the host as unreachable immediately, rather than waiting for the timeout.

@sfackler
Copy link
Owner

That SQLAlchemy case seems to be covering detection disconnections of active sessions, not failing early on pool creation.

@guilhermedelyra
Copy link

guilhermedelyra commented Nov 28, 2024

@sfackler any status on this? 😃

@sfackler
Copy link
Owner

I am still waiting to see an example of another connection pool library that does this.

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

No branches or pull requests

4 participants