ClickHouse
feat[pool]: Expose Close method to facilitate removal of broken connections
Expose Close method to allow connections to be closed if they are in a bad state.
There are situations were the acquired connection is in a broken state but there is no explicitly way to either remove it from the pool or close it.
As a package user I want to have the choice to close and destroy a broken connection.
The backgroundHealthCheck does not check for connection's state therefore it does not remove broken connections.
The snippet below exemplifies how I would use the Close method. After checking the connection state using Ping I can close/destroy in case of an error.
conn, err := pool.Acquire(ctx)
if err != nil {
return nil, err
}
if err := conn.Ping(ctx); err != nil {
conn.Close() <<<<< This connections is broken and should not be put back into the pool using conn.Release()
return nil, level.Error(logger).Log("msg", "failed to ping acquired connection from pool", "err", err)
}
return conn, nil
Please let me know if there is an alternative way of getting the same behaviour.
I've just started the implementation on https://github.com/danieloliveira-shopify/ch-go/pull/1. If this makes sense I can open a PR to merge upstream.
I haven't used the pooling myself. I see there's a few references in that module for Release and Close. It makes sense to me, but I think @ernado will know the intent of this module better.
Feel free to make your PR to upstream so we can follow the progress and review it. I think it makes sense as it is now, maybe just add a test for it.
@ernado does this change make sense in the pool client?
I've never used pooling myself too, so nothing to add here from my side.
Exposing Close method looks ok for me.
The PR is here https://github.com/ClickHouse/ch-go/pull/1054 but I don't know why CI (Lint) is failing. I did not touch any other file. @SpencerTorres @ernado
https://github.com/ClickHouse/ch-go/actions/runs/14060409852/job/39369509293?pr=1054#step:6:33