Closed varrocs closed 11 months ago
Thanks for the report! This is an interesting one.
I write a proper patch if you don't have the resources.
Contributions are always welcome :)
I've just created a pull request: https://github.com/ClickHouse/clickhouse-js/pull/203
Released as 0.2.3. @varrocs, thanks for the fix!
Describe the bug
When using
insert()
with akeep_alive
connection and theasync_insert
option the socket is closed after each insert request.Expected behaviour
The socket is kept open and it is reused for the next insert.
Code example
Error log
See the tcpdump outputs below
Configuration
Environment
ClickHouse server
CREATE TABLE
statements for tables involved:create table test_table(`a` UInt16, `b` UInt16) ENGINE=MergeTree PRIMARY KEY a;
The script that ran the clickhouse server:
Additional notes
e want to store a stream of events in ClickHouse. We don't want to lose data when one of our processes is killed or crashed so we want to push the data as soon as possible and use ClickHouse's async_insert feature to create batches for insert. The socket connection process takes significant time so we'd like to reuse an open connection.
I tried to insert a stream like in this example but when the client is killed the data already sent is lost. It's not committed to the DB.
I suspect that the problem is that destroying the stream eventually closes the socket: https://github.com/ClickHouse/clickhouse-js/blob/main/packages/client-node/src/connection/node_base_connection.ts#L378
I guess the socket shouldn't be closed but somehow returned to the HTTP agent's free socket pool When I commented out this line the socket doesn't get closed with an RST flag but it times out after 3 seconds and gets closed by the server with a FIN flag.
So that gave me an idea to drain the stream instead of closing it so I made this change:
And this works! The socket is not closed, but reused. The data is written to the database even when I kill the client while streaming.
So I suggest to drain stream instead of closing it. Please note that I haven't tried it without keepalive or without async insert.
I write a proper patch if you don't have the resources.
tcpdump output - original
Note the
sequences
The socket close is always initiated by the client lib side I used wireshark but that's hard to include in a bug report.
tcpdump output - fixed version