-
Notifications
You must be signed in to change notification settings - Fork 93
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
Calling connect() after keepalive() #44
Comments
I'm seeing this as well when running the build tests. |
Here's the culprit I'm assuming it was an oversight since I don't see any extra logic at work. |
It is an OpenResty paradigm to create one TCP socket in such a Maybe pgmoon shouldn't free the socket in |
Side effects worse than a nil reference by default? 😏 |
If you feel like this behavior is more desirable than the current one, feel free to propose a PR to this project. |
Looking at lua-resty-redis as a reference, it doesn't appear that the socket should be set to nil. I'll provide a PR soon. |
I found the last paragraph of https://github.com/openresty/lua-resty-redis#set_keepalive gives a pointer that connect is allowed:
Thanks @ttfkam and @thibaultcha |
On the bright side, it turns out the failing tests were due to my build environment. The rockspec file really should have more dependencies defined. On the downside, there are no tests for keepalive I can see. This probably makes sense since the connection behavior is different for OpenResty environments than for straight Lua usage. I use pg_bouncer for my connection pooling, so I don't have a setup handy for it. @gnois, if you checkout my keepalive branch, I'd love to know that it works for you. The only two files you'd need to swap out are pgmoon/init.moon and pgmoon/init.lua |
@ttfkam Works well enough for me. 👍 |
The rockspec is for production installations only, since the format does not support development-only dependencies yet. Development dependencies are to be manually installed; library maintainers have different ways of dealing with them and/or simply document them in the README. Feel free to send a PR improving the process for this library if you feel a need for it. |
Still getting used to Lua. Thank you for the explanation. |
Associating the pull request with this issue. #46 |
@ttfkam |
I put up a pull request months ago with what I saw as the best solution at the time, but it's open source; you can't force anyone to actually accept the patch if they don't want to. Until @leafo accepts the patch or fixes on his own as he sees fit, you'll have to simply stop using pgmoon's implementation of keepalive(). As a workaround, you can use pgbouncer to manage connection pooling rather than relying on the database driver to do so. |
@ttfkam |
Yes, #46 |
This used to work in 1.6 but no longer in 1.7 or 1.8, with error
.../pgmoon/init.lua:194: attempt to index field 'sock' (a nil value)
File app/db.lua
File app/handler.lua
I found that the self.sock handle is set to nil after keepalive().
Can we call connect() after keepalive() if they are within the same request?
Thanks!
The text was updated successfully, but these errors were encountered: