AutoCloseable ClickHouseClient and async execute

[ahmedriza]

Describe the bug

This is not so much a bug as much as trying to understand how to use the ClickHouseClient properly in an asynchronous context.

The ClickHouseClient is an AutoCloseable and we are supposed to use in the following manner

Code example

String sql = "...";
Consumer<ClickHouseResponse> consumer = ...;
try(ClickHouseClient client = ClickHouseClient.newInstance(this.clickHouseNode.getProtocol())) {
      ClickHouseRequest<?> request = client.read(this.clickHouseNode);
      LOG.debug("Executing ClickHouse SQL: {}", sql);
      return request
          .format(ClickHouseFormat.RowBinaryWithNamesAndTypes)
          .query(sql)
          .execute()
          .thenAcceptAsync(consumer);
    } 

Environment

• Client version: 0.4.4
• Language version: JDK 17
• OS: RHEL 8

The issue is that the try-with clause will close the ClickHouseClient before the async calls complete and this will obviously lead to those calls failing. What's the correct usage of the ClickHouseClient in this context?

[mbaksheev]

The AutoCloseable is not useful in case of async processing. I would suggest to implement some wrapper function in order to close client after async call complete. E.g.

 public CompletableFuture<ClickHouseResponse> executeAsync(ClickHouseNode node, String sql) {
        ClickHouseClient client = ClickHouseClient.newInstance(node.getProtocol());
        try {
            ClickHouseRequest<?> request = client.read(node);
            return request
                    .format(ClickHouseFormat.RowBinaryWithNamesAndTypes)
                    .query(sql)
                    .execute()
                    .whenComplete((response, error) -> {
                        // close the client when async call complete
                        client.close();
                    });

        } catch (Exception e){
            client.close();
            return CompletableFuture.failedFuture(e);
        }
    }

And then call it:

executeAsync(this.clickHouseNode, sql)
    .thenApply(clickHouseResponse -> {
        // do useful stuff with response
 });

But since you are using async calls, you probably want to execute bunch of async calls. So you can just send multiple async requests inside the try-with-resources block with the same client instance and wait for all of them inside the block.

[mtrienis]

@mbaksheev do you also need to close the response similar to the example here

Also, if I just want to do a batch async insert as fast as possible using the HTTP protocol, is this still the recommended approach with the Java client (i.e. using the ClickHousePipedOutputStream)? Assuming I replace the future.get() with future.whenComplete(...) as you mentioned above.