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

Setting the network stream read timeout based on parameters LoginTimeout #165

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

anat0li
Copy link
Contributor

@anat0li anat0li commented Jan 10, 2020

Currently, network stream read timeout is -1 which means unlimited. That makes an application hang or freeze because of the driver if the server does not respond. In my particular case, it happened while connecting to some corrupted Sybase 16.0 installation under solaris platform. Maybe somebody knowing the driver code in more details can suggest some better solution for this problem. But the suggested simple modification makes the driver behaviour more natural resulting in AseException : "Pool timed out trying to reserve a connection" instead of keeping the application frozen forever.

@senseibaka
Copy link
Collaborator

Yes, I think there'll need to be a different way, rather than setting the network stream's timeout wholesale.

The reason I say that is because you can have an idle, long-lived connection to the server that shouldn't be killed, for example:

  • Connection pool has a bunch of idle connections that are perfectly useable
  • Some people do some long-running query, that is expected to be long-running

Maybe it's possible to pass a cancellation token into the connection.Login() method?

@anat0li
Copy link
Contributor Author

anat0li commented Jan 13, 2020

IMO if people do some query, that is expected to be long-running, they have to pass some substantial timeout value as a parameter. As for cancellation token, the question is how it is supposed to be triggered? The most likely it is just waiting for some limited timeout period and then triggering a cancellation token. Again such functionality can be parameterized by some timeout value. That timeout parameter value can be named differently from LoginTimeout or IdleTimeout. I just think it is better to set some explicit limit for waiting than be waiting forever. And again, explicitly setting the timeout to -1 results in the current behavior, but at least a user has some options.

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.

2 participants