ConnOpenStrategy should trigger on every query and between life connections, to implement load balance

[mhlywxl]

Describe the bug

ConnOpenStrategy should trigger on every query and between life connections，this my expect，but now i can only set the ConnMaxLifetime in a smaller interval，such as 3 seconds，and after 3 seconds，the connection idle，and trigger dialStrategy. i wish clickhouse-go should choose connections between the life and good connections. by th way, it should classify the connections by the Addr array. for example, my Addr= [127.0.0.1:29100,127.0.0.2:29100], when ConnOpenStrategy=ConnOpenRoundRobin and 127.0.0.1 and 127.0.0.2 is all good, every time i call connection to exec sql, it should round between 127.0.0.1 and 127.0.0.2.

maybe in the clickhouse-go inside, 127.0.0.1 and 127.0.0.2 has one more connection instance because of MaxOpenConns, but to the caller, he also can round between 127.0.0.1 and 127.0.0.2.

on the whole，i wish clickhouse-go can matain good addrs and good connections pool inside, it shoud clean bad or idle connections inside periodly. ConnOpenStrategy trigger on every query instead of dia.

Code example

        conn, err := clickhouse.Open(&clickhouse.Options{
		Addr: []string{
			"127.0.0.1:29100",
			"127.0.0.2:29100",
		},
		ConnOpenStrategy: clickhouse.ConnOpenRoundRobin,
		Debug:            true,
		Auth: clickhouse.Auth{
			Database: "default",
			Username: "xx",
			Password: "xx",
		},
		ConnMaxLifetime: 3 * time.Second,
	})
	if err != nil {
		t.Log(err)
	}
	t.Log("=====================")
	for i := 0; i < 3; i++ {
		v, err := conn.ServerVersion()
		if err != nil {
			t.Log(err)
		}
		t.Log(v.String())
	}
	t.Log("=====================")
	for i := 0; i < 3; i++ {
		v, err := conn.ServerVersion()
		if err != nil {
			t.Log(err)
		}
		t.Log(v.String())
		t.Log("sleep 5 seconds")
		time.Sleep(5 * time.Second)
	}

log

    main_test.go:191: =====================
[clickhouse][conn=1][127.0.0.1:29100][handshake] ->  0.0.0
[clickhouse][conn=1][127.0.0.1:29100][handshake] downgrade client proto
[clickhouse][conn=1][127.0.0.1:29100][handshake] <- ClickHouse (127.0.0.1) server version 22.3.3 revision 54455 (timezone Asia/Shanghai)
    main_test.go:197: ClickHouse (127.0.0.1) server version 22.3.3 revision 54455 (timezone Asia/Shanghai)
    main_test.go:197: ClickHouse (127.0.0.1) server version 22.3.3 revision 54455 (timezone Asia/Shanghai)
    main_test.go:197: ClickHouse (127.0.0.1) server version 22.3.3 revision 54455 (timezone Asia/Shanghai)
    main_test.go:199: =====================
    main_test.go:205: ClickHouse (127.0.0.1) server version 22.3.3 revision 54455 (timezone Asia/Shanghai)
    main_test.go:206: sleep 5 seconds
[clickhouse][conn=2][127.0.0.2:29100][handshake] ->  0.0.0
[clickhouse][conn=2][127.0.0.2:29100][handshake] downgrade client proto
[clickhouse][conn=2][127.0.0.2:29100][handshake] <- ClickHouse (127.0.0.2) server version 22.3.3 revision 54455 (timezone Asia/Shanghai)
    main_test.go:205: ClickHouse (127.0.0.2) server version 22.3.3 revision 54455 (timezone Asia/Shanghai)
    main_test.go:206: sleep 5 seconds
[clickhouse][conn=3][127.0.0.1:29100][handshake] ->  0.0.0
[clickhouse][conn=3][127.0.0.1:29100][handshake] downgrade client proto
[clickhouse][conn=3][127.0.0.1:29100][handshake] <- ClickHouse (127.0.0.1) server version 22.3.3 revision 54455 (timezone Asia/Shanghai)
    main_test.go:205: ClickHouse (127.0.0.1) server version 22.3.3 revision 54455 (timezone Asia/Shanghai)
    main_test.go:206: sleep 5 seconds

Expected behaviour

every time call conn.ServerVersion()
it should print different ip from last time

[jkaflik]

Hi @mhlywxl,

In the first for-loop, you reuse the same connection. The first iteration of the second loop also reuses the same connection.
It's because of availability of connections in the connection pool. Perhaps you want to decrease max connections in the options?

Do I understand correctly you would like to execute each query on the different server?

[mhlywxl]

@jkaflik hi

Yes, I would like to execute each query on the different server, but the max connections and maxLifeTime config should not affect the result, I wish first-loop log shoud be like the second-loop.

In the first for-loop, I know it reuse, reuse is no problem, the problem is I wish it can loop reuse the different connections,not the same connection,because i wish to load balance between different ip host. On the other hand, I config the roundRobin strategy, so first-loop log shoud be like the second-loop.

By the way, connections in the connection pool is useful, I sugguest to implement load balance base on this.

for example:

scene1

Config
Addrs: ip1,ip2,ip3
MaxConnections: 4

then the pool can be: connection1(ip1),connection2(ip2),connection3(ip3),connection4(ip1)

when client call:
1 query: use connection1(ip1)
2 query: use connection2(ip2)
3 query: use connection3(ip3)
4 query: use connection1(ip1)
5 query: use connection2(ip2)

scene2

Config:
Addrs: ip1,ip2,ip3
MaxConnections: 6

then the pool can be: connection1(ip1),connection2(ip2),connection3(ip3),connection4(ip1),connection5(ip2),connection6(ip3)

when client call:
1 query: use connection1(ip1)
2 query: use connection2(ip2)
3 query: use connection3(ip3)
4 query: use connection4(ip1)
5 query: use connection5(ip2)
6 query: use connection6(ip3)
7 query: use connection1(ip1)
8 query: use connection2(ip2)
9 query: use connection3(ip3)

this can implement both availability of connections in the connection pool and load balance on different hosts

[jkaflik]

@mhlywxl would you like to contribute?