fix(*) cassandra contact points initialization #4378
Conversation
|
Wondering whether the cassandra code is the right place to fix this. Wouldn't the dns client be the right place to fix it? Also wondering, doesn't this same issue exist for Postgres? Why do we connect to Cassandra in this phase, but not to Postgres? |
|
@Tieske Based on my tests of rc1 postgres does not suffer the same issue |
|
@Tieske the question is that do we want to have this hack available anywhere or just here. I guess you could perhaps inject such hack at some point to your dns client if we need it work anywhere. Postgres does not run anything on init, not sure if it should test connection there(?). Because we are about to release the rc2 soon, I wanted to isolate this hack so that it has a smaller surface area (and no need to update dependencies at this point of release cycle). |
Both Postgres and Cassandra strategies will connect to their respective instances during |
|
(And the tests run in |
C* doesn't initialize contact points correctly when the contact points need to be resolved with DNS. DNS on the other hand uses Nginx cosockets that cannot be used with init phase. Nginx cosockets are not fully compatible with lua sockets. This PR is a quick fix for the issue.
bungle
force-pushed
the
fix/cassandra-contact-points
branch
from
9a8b3d2 to
1159fcc
Compare
| @@ -14,7 +14,77 @@ function CassandraConnector.new(kong_config) | |||
| do | |||
| -- Resolve contact points before instantiating cluster, since the | |||
| -- driver does not support hostnames in the contact points list. | |||
|
|
|||
| -- TODO: this is an ugly hack to force lua sockets on a third party library | |||
There was a problem hiding this comment.
May be worth being more explicit here:
- we force sync option for resty.dns.client to not spawn a timer
- we force LuaSocket for resty.dns.resolver to not spawn a cosocket in init_by_lua
Can add at merge time, in fact let me take care of it, no worries 👍
| }) | ||
| end | ||
|
|
||
| ngx.socket.udp = function(...) |
There was a problem hiding this comment.
Note for posterity:
ideally we should add support for ngx.socket.udp to lua-resty-socket and avoid duplicating the fallback logic there and here (lua-resty-socket has its own test suite and benefits other projects).
There was a problem hiding this comment.
Oh yes, looks like it already has all things covered. Lets add dependency to it with 1.1+.
C* doesn't initialize contact points correctly when the contact points need to be resolved with DNS. DNS on the other hand uses Nginx cosockets that cannot be used with init phase. Nginx cosockets are not fully compatible with lua sockets. This PR is a quick fix for the issue. From #4378 Signed-off-by: Thibault Charbonnier <thibaultcha@me.com>
Summary
C* doesn't initialize contact points correctly when the contact points need to be resolved with DNS. DNS on the other hand uses Nginx cosockets that cannot be used with init phase. Nginx cosockets are not fully compatible with lua sockets. This PR is a quick fix for the issue.
This is based on
nextto prevent some merge conflicts. I can rebase on master if that is needed.