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

SNOW-1229299 Honor PDO Timeout Setting on Connections #170

Closed
sfc-gh-kwagner opened this issue Jan 22, 2019 · 12 comments
Closed

SNOW-1229299 Honor PDO Timeout Setting on Connections #170

sfc-gh-kwagner opened this issue Jan 22, 2019 · 12 comments
Assignees
Labels
bug status-triage_done Initial triage done, will be further handled by the driver team

Comments

@sfc-gh-kwagner
Copy link
Contributor

When a new connection is established, we should honor the connection timeout value specified by the PDO driver

@plsoucy-tapclicks
Copy link

Is there any update on this? We are seeing an issue where the PDO timeout would be helpful if respected, and would like to know if this has been addressed in the last 3 years. Thanks!

@waynehamadi
Copy link

Any update on this ?

@sfc-gh-dszmolka
Copy link
Contributor

hi - there's been a couple releases since this issue was raised, so closing this issue for now. If it's still a problem, do comment please and I'll reopen and we can look.

@aarefjev
Copy link

aarefjev commented Mar 8, 2024

hi - there's been a couple releases since this issue was raised, so closing this issue for now. If it's still a problem, do comment please and I'll reopen and we can look.

Tested just now and issue still present :(
Very annoying as I cannot use PDO to export large chunks of data without some code workarounds.

@sfc-gh-dszmolka sfc-gh-dszmolka added the status-triage Issue is under initial triage label Mar 8, 2024
@sfc-gh-dszmolka
Copy link
Contributor

did some tests and it appears that this is not so well documented 😓 but apparently we can rely on various timeout settings :

        {"logintimeout",        "300",        0},
        {"maxhttpretries",      "7",          0},
        {"retrytimeout",        "300",        0}

starting from v2.0.1 and onwards, problem is that they stil don't seem to be taking effect although the driver correctly logs their altered value when using like

$dbh = new PDO("snowflake:account=$account;logintimeout=3;retrytimeout=4;maxhttpretries=5", $user, $password);

we're taking a look further.

@sfc-gh-dszmolka sfc-gh-dszmolka added status-triage_done Initial triage done, will be further handled by the driver team and removed status-triage Issue is under initial triage labels Mar 11, 2024
@sfc-gh-dszmolka sfc-gh-dszmolka changed the title Honor PDO Timeout Setting on Connections SNOW-1229299 Honor PDO Timeout Setting on Connections Mar 11, 2024
@bnd-tapclicks
Copy link

Could you please provide an estimated timeframe for the resolution? It would be greatly appreciated. Thank you.

@sfc-gh-dszmolka
Copy link
Contributor

At this moment I can share the following updates from the dev team, hopefully clarifies some bits:

  • retrytimeout can't be set to lower than 300 so in the test case it was set to 4 but actually 300 is used.
  • maxhttpretries can't be set to lower than 7 so set it to 5 in the test, and actually 7 being used, it is expected
  • network_timeout currently there is no way to set network_timeout in PHP driver, while the actual timeout being used is the lower one of network_timeout and retrytimeout and the default value for network_timeout of 0 is considered unlimited . so if network_timeout=0 and retrytimeout=300, the actual timeout would be 300
  • login timeout: it can be set to 3 and expected to work, we're still investigating why it did not.
  • unfortunately for now retrytimeout only works for login/query requests, chunk downloading is using hardcoded timeout of 60 seconds.

so hard to estimate the fix, because partly it works as expected (just I used unexpected values for my test), partly it really seems to be bug-ish, partly is an enhancement for e.g. chunk downloading which does not have a controllable timeout today

i'll post any new information in the same thread and thank you for bearing with us !

@sfc-gh-dszmolka
Copy link
Contributor

This is confirmed to be a bug in the underlying libsnowflakeclient.

Currently the retry timeout (or network timeout) would only work as expected when it's longer than the default timeout (CURLOPT_CONNECTTIMEOUT, CURLOPT_TIMEOUT) or there is no network issue and the server send back a retry-able http error.

Driver team will make fix on the libsnowflakeclient side, both the timeout issue and the retry behavior for chunk downloading.

@sfc-gh-dszmolka
Copy link
Contributor

PR (in the underlying libsnowflakeclient) snowflakedb/libsnowflakeclient#683

@sfc-gh-dszmolka sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-pr_pending_merge A PR is made and is under review labels Mar 20, 2024
@sfc-gh-dszmolka
Copy link
Contributor

sfc-gh-dszmolka commented Mar 21, 2024

PR merged and will be released with the next version of the PHP driver, which first needs to be rebased on the new release from libsnowflakeclient which actually contains the fix. So at this very moment, I cannot confirm which PHP driver version will actually have the fixed code.

Have no timeline on this at this moment but will update this thread.

In the meantime, PHP driver's README has been updated with some description about the available timeout values and setting them.

@sfc-gh-dszmolka
Copy link
Contributor

sfc-gh-dszmolka commented Mar 28, 2024

quick update: fixed libsnowflakeclient now released and we're targeting 2024 April (~end of month) to release the new PHP driver based on that.

edit: fix for PHP driver based on the libsnowflakeclient fix: #376

@sfc-gh-dszmolka
Copy link
Contributor

fix is available with the fresh release of Snowflake PHP driver v2.0.3

@sfc-gh-dszmolka sfc-gh-dszmolka removed the status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

6 participants