Proxy info messages to the adapter

[ruslandoga]

Context: #315 (comment)

[ruslandoga]

I'm still passing the state to the callback since the monitor ref / socket ref would be there, and they are probably needed to match on the incoming message.

[ruslandoga]

Like here.

[ruslandoga]

I'm using this just to "sync" with the connection process, i.e. wait until it has processed all the messages. Maybe there is a better way? I know some people use :sys.get_state/1 (e.g. :sys.get_state(conn) in this case) but don't know if it's any better.

[josevalim]

💚 💙 💜 💛 ❤️

[ruslandoga]

Seems like I forgot to run formatter on examples/ directory.

[josevalim]

I just did it in main, no worries. But we also forgot to document handle_info as a callback and list it as optional. Can you please send a PR to add that too? Sorry.

[ruslandoga]

Sure!

[ruslandoga]

Hm... I tried handle_info/2 out in plausible/ch#212 and something seems off. DBConnection.execute gets an old connection even though it happens strictly after reconnect.

connect: #Port<0.10>
handshake (in connect): "select 1" on #Port<0.10>
-- first spawn --
handle_execute: "select 1 + 1" on #Port<0.10>
handle_info: #Port<0.10> closed :: {:disconnect, %Mint.TransportError{reason: :closed}}
disconnect: #Port<0.10>
connect: #Port<0.12>
handshake (in connect): "select 1" on #Port<0.12>
-- second spawn --
handle_execute: "select 1 + 1" on #Port<0.10> <-- old conn

I'll look into it more tomorrow, but first thought is ETS holds onto the old conn for some reason and checkout happens before pool_update(new_state) even though connection completes before the checkout. It might be time to finally learn how DBConnection does its magic :)

---

UPDATE: It seems like the ETS table is not updated on disconnect, so the very next query gets a stale conn.

[ruslandoga]

👋 @josevalim

It doesn't seem like handle_info is of any use since the internal disconnect event doesn't remove "stale" connections from the ETS table. I think I will need to explore other solutions.

Should I revert this PR?

[josevalim]

@ruslandoga it is up to you. Couldn't we allow :disconnect to be returned from handle_info and then have that remove it from the table? Another option is to do a checkout, like we do on ping, and then it will be properly disconnected. But in such case we need to be very clear that handle_info in our case is a relatively expensive operation, as it checks out, and it must be used only in cases like this.

[ruslandoga]

ReqCH gave me an idea that maybe DBConnection is not necessary for a ClickHouse client.

And it doesn't seem like any other adapter needs a handle_info since that issue hasn't been brought up before.
So I'll revert it to avoid complicating DBConnection unnecessarily.

Sorry!