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

Current ODBC liveness checks may be inadequate #9347

Closed
NattyNarwhal opened this issue Aug 15, 2022 · 3 comments
Closed

Current ODBC liveness checks may be inadequate #9347

NattyNarwhal opened this issue Aug 15, 2022 · 3 comments

Comments

@NattyNarwhal
Copy link
Member

Description

In GH-6805, I implemented liveness checks for PDO_ODBC, like procedural ODBC and other PDO drivers. I had used the same technique that procedural ODBC does of SQLGetInfo(SQL_DATA_SOURCE_READ_ONLY), but I suspect this is not the best idea in retrospect for either ODBC driver.

The reason why I mention this is because it seemed the proper ODBC facility for this, SQL_ATTR_CONNECTION_DEAD (introduced in a recent-ish version of ODBC), seemed to have spotty support. The driver I support for my users (Db2 for i) didn't seem to support it at that time. This meant we had to use this method, which isn't semantically correct (so there are no guarantees it works like we're using it), but seemed to work.

The problem now is that a new version of the Db2 for i driver both now supports this and caches the SQL_DATA_SOURCE_READ_ONLY value, making it useless for determining if a connection is alive. However, I don't want to be selfish here by only caring about the driver I have to deal with. There are other drivers we should be considerate about, though it's hard since their behaviour and ODBC version levels aren't really well documented, especially for closed-source ones.

I wonder if the best course of action would be to try SQL_ATTR_CONNECTION_DEAD, then fall back to the previous strategy, which seems to work OK for most drivers.

However, I don't want to create a mess like ibm_db2 has, where there are multiple strategies the user picks from, some of which won't work anymore. It would be a massive cognitive burden on the user. Using different strategies transparently may not work either - what if one hangs because the connection is dead? That, and the performance impact may cancel out the benefits of things like persistent connections.

I'm raising this as a discussion here, since I think @cmb69 has a lot more experience with other drivers, as well as what could be an appropriate solution towards liveness checks that work consistently.

@cmb69
Copy link
Member

cmb69 commented Aug 16, 2022

Ah, good point. Thank you!

I wonder if the best course of action would be to try SQL_ATTR_CONNECTION_DEAD, then fall back to the previous strategy, which seems to work OK for most drivers.

Yeah, I think that is the best way forward for now. If we target "master" only, and apply that change soon, chances are that any serious issue with this approach would be reported before GA, in which case we could still look for further improvements.

Are you interested in providing a PR?

@NattyNarwhal
Copy link
Member Author

I'll try to write a PR. I should be able to test with the Db2i driver, and hopefully other ODBC users will follow. That said, I worry a lot of weird driver users might be trailing edge with PHP - I can at least try to get Db2i users I'm supporting to try 8.2.

@NattyNarwhal
Copy link
Member Author

I've implemented this in a PR and tested it with Db2i, though unfortunately, there's an issue with the driver in one specific scenario with one specific driver - but I think that's a me-annoying-the-driver-maintainer problem, not a PHP problem.

@cmb69 cmb69 closed this as completed in f3a14d1 Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants