Skip to content

Commit 04b9c4b

Browse files
committed
Fix slowness in performance mode and failover not working when protocol is unsupported
1 parent 9b21161 commit 04b9c4b

File tree

8 files changed

+90
-27
lines changed

8 files changed

+90
-27
lines changed

clickhouse-cli-client/README.md

+5-5
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ This is a thin wrapper of ClickHouse native command-line client. It provides an
66
- native CLI client instead of pure Java implementation
77
- an example of implementing SPI defined in `clickhouse-client` module
88

9-
Either [clickhouse-client](https://clickhouse.com/docs/en/interfaces/cli/) or [docker](https://docs.docker.com/get-docker/) must be installed prior to use. And it's important to understand that this module uses sub-process(in addition to threads) and file-based streaming, meaning 1) it's not as fast as native CLI client or pure Java implementation, although it's close in the case of dumping and loading data; and 2) it's not suitable for scenarios like dealing with many queries in short period of time.
9+
Either [clickhouse](https://clickhouse.com/docs/en/interfaces/cli/) or [docker](https://docs.docker.com/get-docker/) must be installed prior to use. And it's important to understand that this module uses sub-process(in addition to threads) and file-based streaming, meaning 1) it's not as fast as native CLI client or pure Java implementation, although it's close in the case of dumping and loading data; and 2) it's not suitable for scenarios like dealing with many queries in short period of time.
1010

1111
## Limitations and Known Issues
1212

13-
- Only `max_result_rows` and `result_overflow_mode` two settings are currently supported
13+
- Only `max_result_rows`, `result_overflow_mode` and `readonly` 3 settings are currently supported
1414
- ClickHouseResponseSummary is always empty - see ClickHouse/ClickHouse#37241
1515
- Session is not supported - see ClickHouse/ClickHouse#37308
1616

@@ -28,10 +28,10 @@ Either [clickhouse-client](https://clickhouse.com/docs/en/interfaces/cli/) or [d
2828
## Examples
2929

3030
```java
31-
// make sure 'clickhouse-client' or 'docker' is in PATH before you start the program
31+
// make sure 'clickhouse' or 'docker' is in PATH before you start the program
3232
// alternatively, configure CLI path in either Java system property or environment variable, for examples:
33-
// CHC_CLICKHOUSE_CLI_PATH=/path/to/clickhouse-client CHC_DOCKER_CLI_PATH=/path/to/docker java MyProgram
34-
// java -Dchc_clickhouse_cli_path=/path/to/clickhouse-client -Dchc_docker_cli_path=/path/to/docker MyProgram
33+
// CHC_CLICKHOUSE_CLI_PATH=/path/to/clickhouse CHC_DOCKER_CLI_PATH=/path/to/docker java MyProgram
34+
// java -Dchc_clickhouse_cli_path=/path/to/clickhouse -Dchc_docker_cli_path=/path/to/docker MyProgram
3535

3636
// clickhouse-cli-client uses TCP protocol
3737
ClickHouseProtocol preferredProtocol = ClickHouseProtocol.TCP;

clickhouse-cli-client/src/main/java/com/clickhouse/client/cli/ClickHouseCommandLine.java

+8-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.io.IOException;
99
import java.io.OutputStream;
1010
import java.io.UncheckedIOException;
11+
import java.net.ConnectException;
1112
import java.nio.charset.StandardCharsets;
1213
import java.nio.file.Files;
1314
import java.nio.file.Path;
@@ -93,7 +94,7 @@ static void dockerCommand(ClickHouseConfig config, String hostDir, String contai
9394
cli = DEFAULT_DOCKER_CLI_PATH;
9495
}
9596
if (!check(timeout, cli, DEFAULT_CLI_ARG_VERSION)) {
96-
throw new IllegalStateException("Docker command-line is not available: " + cli);
97+
throw new UncheckedIOException(new ConnectException("Docker command-line is not available: " + cli));
9798
} else {
9899
commands.add(cli);
99100
}
@@ -111,7 +112,7 @@ static void dockerCommand(ClickHouseConfig config, String hostDir, String contai
111112
DEFAULT_CLI_ARG_VERSION)
112113
&& !check(timeout, cli, "run", "--rm", "--name", str, "-v", hostDir + ':' + containerDir,
113114
"-d", img, "tail", "-f", "/dev/null")) {
114-
throw new IllegalStateException("Failed to start new container: " + str);
115+
throw new UncheckedIOException(new ConnectException("Failed to start new container: " + str));
115116
}
116117
}
117118
}
@@ -122,7 +123,7 @@ static void dockerCommand(ClickHouseConfig config, String hostDir, String contai
122123
} else { // create new container for each query
123124
if (!check(timeout, cli, "run", "--rm", img, DEFAULT_CLICKHOUSE_CLI_PATH, DEFAULT_CLIENT_OPTION,
124125
DEFAULT_CLI_ARG_VERSION)) {
125-
throw new IllegalStateException("Invalid ClickHouse docker image: " + img);
126+
throw new UncheckedIOException(new ConnectException("Invalid ClickHouse docker image: " + img));
126127
}
127128
commands.add("run");
128129
commands.add("--rm");
@@ -235,6 +236,10 @@ static Process startProcess(ClickHouseNode server, ClickHouseRequest<?> request)
235236
if (value != null) {
236237
commands.add("--result_overflow_mode=".concat(value.toString()));
237238
}
239+
value = settings.get("readonly");
240+
if (value != null) {
241+
commands.add("--readonly=".concat(value.toString()));
242+
}
238243
if ((boolean) config.getOption(ClickHouseCommandLineOption.USE_PROFILE_EVENTS)) {
239244
commands.add("--print-profile-events");
240245
commands.add("--profile-events-delay-ms=-1");

clickhouse-cli-client/src/main/java/com/clickhouse/client/cli/config/ClickHouseCommandLineOption.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
public enum ClickHouseCommandLineOption implements ClickHouseOption {
99
/**
1010
* ClickHouse native command-line client path. Empty value is treated as
11-
* 'clickhouse-client'.
11+
* 'clickhouse'.
1212
*/
1313
CLICKHOUSE_CLI_PATH("clickhouse_cli_path", "",
14-
"ClickHouse native command-line client path, empty value is treated as 'clickhouse-client'"),
14+
"ClickHouse native command-line client path, empty value is treated as 'clickhouse'"),
1515
/**
1616
* ClickHouse docker image. Empty value is treated as
1717
* 'clickhouse/clickhouse-server'.

clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseClient.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ static ClickHouseInputStream getResponseInputStream(ClickHouseConfig config, Inp
151151

152152
/**
153153
* Gets piped input stream for reading data from response asynchronously. When
154-
* {@code config} is null or {@code config.isAsync()} is faluse, this method is
154+
* {@code config} is null or {@code config.isAsync()} is false, this method is
155155
* same as
156156
* {@link #getResponseInputStream(ClickHouseConfig, InputStream, Runnable)}.
157157
*

clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseClientBuilder.java

+14-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.clickhouse.client;
22

33
import java.io.Serializable;
4+
import java.io.UncheckedIOException;
5+
import java.net.ConnectException;
46
import java.util.HashMap;
57
import java.util.Map;
68
import java.util.Objects;
@@ -27,7 +29,7 @@
2729
*/
2830
public class ClickHouseClientBuilder {
2931
/**
30-
* Dummy client which is only used {@link Agent}.
32+
* Dummy client which is only used by {@link Agent}.
3133
*/
3234
static class DummyClient implements ClickHouseClient {
3335
static final ClickHouseConfig CONFIG = new ClickHouseConfig();
@@ -40,7 +42,10 @@ public boolean accept(ClickHouseProtocol protocol) {
4042

4143
@Override
4244
public CompletableFuture<ClickHouseResponse> execute(ClickHouseRequest<?> request) {
43-
return CompletableFuture.completedFuture(ClickHouseResponse.EMPTY);
45+
CompletableFuture<ClickHouseResponse> future = new CompletableFuture<>();
46+
future.completeExceptionally(
47+
new ConnectException("No client available for connecting to: " + request.getServer()));
48+
return future;
4449
}
4550

4651
@Override
@@ -55,7 +60,7 @@ public void close() {
5560

5661
@Override
5762
public boolean ping(ClickHouseNode server, int timeout) {
58-
return true;
63+
return false;
5964
}
6065
}
6166

@@ -166,6 +171,11 @@ ClickHouseResponse retry(ClickHouseRequest<?> sealedRequest, Throwable cause, in
166171
}
167172

168173
ClickHouseResponse handle(ClickHouseRequest<?> sealedRequest, Throwable cause) {
174+
// in case there's any recoverable exception wrapped by UncheckedIOException
175+
if (cause instanceof UncheckedIOException && cause.getCause() != null) {
176+
cause = ((UncheckedIOException) cause).getCause();
177+
}
178+
169179
try {
170180
int times = sealedRequest.getConfig().getFailover();
171181
if (times > 0) {
@@ -352,7 +362,7 @@ public ClickHouseClient build() {
352362
}
353363
}
354364

355-
if (client == null) {
365+
if (client == null && !agent) {
356366
throw new IllegalStateException(
357367
ClickHouseUtils.format("No suitable ClickHouse client(out of %d) found in classpath for %s.",
358368
counter, nodeSelector));

clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseNodes.java

+16
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,10 @@ public static ClickHouseNodes of(String enpoints, Map<?, ?> options) {
314314
* Load balancing tags for filtering out nodes.
315315
*/
316316
protected final ClickHouseNodeSelector selector;
317+
/**
318+
* Flag indicating whether it's single node or not.
319+
*/
320+
protected final boolean singleNode;
317321
/**
318322
* Template node.
319323
*/
@@ -358,8 +362,11 @@ protected ClickHouseNodes(Collection<ClickHouseNode> nodes, ClickHouseNode templ
358362
n.setManager(this);
359363
}
360364
if (autoDiscovery) {
365+
this.singleNode = false;
361366
this.discoveryFuture.getAndUpdate(current -> policy.schedule(current, ClickHouseNodes.this::discover,
362367
(int) template.config.getOption(ClickHouseClientOption.NODE_DISCOVERY_INTERVAL)));
368+
} else {
369+
this.singleNode = nodes.size() == 1;
363370
}
364371
this.healthCheckFuture.getAndUpdate(current -> policy.schedule(current, ClickHouseNodes.this::check,
365372
(int) template.config.getOption(ClickHouseClientOption.HEALTH_CHECK_INTERVAL)));
@@ -472,6 +479,15 @@ protected ClickHouseNode get() {
472479
return apply(selector);
473480
}
474481

482+
/**
483+
* Checks whether it's single node or not.
484+
*
485+
* @return true if it's single node; false otherwise
486+
*/
487+
public boolean isSingleNode() {
488+
return singleNode;
489+
}
490+
475491
@Override
476492
public ClickHouseNode apply(ClickHouseNodeSelector t) {
477493
lock.readLock().lock();

clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseConnectionImpl.java

+27-8
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@
2727

2828
import com.clickhouse.client.ClickHouseChecker;
2929
import com.clickhouse.client.ClickHouseClient;
30+
import com.clickhouse.client.ClickHouseClientBuilder;
3031
import com.clickhouse.client.ClickHouseColumn;
3132
import com.clickhouse.client.ClickHouseConfig;
3233
import com.clickhouse.client.ClickHouseFormat;
3334
import com.clickhouse.client.ClickHouseNode;
3435
import com.clickhouse.client.ClickHouseNodeSelector;
36+
import com.clickhouse.client.ClickHouseNodes;
3537
import com.clickhouse.client.ClickHouseParameterizedQuery;
3638
import com.clickhouse.client.ClickHouseRecord;
3739
import com.clickhouse.client.ClickHouseRequest;
@@ -218,16 +220,33 @@ public ClickHouseConnectionImpl(ConnectionInfo connInfo) throws SQLException {
218220
jdbcConf = connInfo.getJdbcConfig();
219221

220222
autoCommit = !jdbcConf.isJdbcCompliant() || jdbcConf.isAutoCommit();
221-
222-
ClickHouseNode node = connInfo.getServer();
223-
log.debug("Connecting to: %s", node);
224-
225223
jvmTimeZone = TimeZone.getDefault();
226224

227-
client = ClickHouseClient.builder().options(ClickHouseDriver.toClientOptions(connInfo.getProperties()))
228-
.defaultCredentials(connInfo.getDefaultCredentials())
229-
.nodeSelector(ClickHouseNodeSelector.of(node.getProtocol())).build();
230-
clientRequest = client.connect(node);
225+
ClickHouseClientBuilder clientBuilder = ClickHouseClient.builder()
226+
.options(ClickHouseDriver.toClientOptions(connInfo.getProperties()))
227+
.defaultCredentials(connInfo.getDefaultCredentials());
228+
ClickHouseNodes nodes = connInfo.getNodes();
229+
final ClickHouseNode node;
230+
if (nodes.isSingleNode()) {
231+
try {
232+
node = nodes.apply(nodes.getNodeSelector());
233+
} catch (Exception e) {
234+
throw SqlExceptionUtils.clientError("Failed to get single-node", e);
235+
}
236+
client = clientBuilder.nodeSelector(ClickHouseNodeSelector.of(node.getProtocol())).build();
237+
clientRequest = client.connect(node);
238+
} else {
239+
log.debug("Selecting node from: %s", nodes);
240+
client = clientBuilder.nodeSelector(nodes.getNodeSelector()).build();
241+
clientRequest = client.connect(nodes);
242+
try {
243+
node = clientRequest.getServer();
244+
} catch (Exception e) {
245+
throw SqlExceptionUtils.clientError("No healthy node available", e);
246+
}
247+
}
248+
249+
log.debug("Connecting to: %s", node);
231250
ClickHouseConfig config = clientRequest.getConfig();
232251
String currentUser = null;
233252
TimeZone timeZone = null;

clickhouse-jdbc/src/main/java/com/clickhouse/jdbc/internal/ClickHouseJdbcUrlParser.java

+17-4
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,10 @@
1212
import com.clickhouse.client.ClickHouseUtils;
1313
import com.clickhouse.client.config.ClickHouseClientOption;
1414
import com.clickhouse.client.config.ClickHouseDefaults;
15-
import com.clickhouse.client.logging.Logger;
16-
import com.clickhouse.client.logging.LoggerFactory;
1715
import com.clickhouse.jdbc.JdbcConfig;
1816
import com.clickhouse.jdbc.SqlExceptionUtils;
1917

2018
public class ClickHouseJdbcUrlParser {
21-
private static final Logger log = LoggerFactory.getLogger(ClickHouseJdbcUrlParser.class);
22-
2319
public static class ConnectionInfo {
2420
private final ClickHouseCredentials credentials;
2521
private final ClickHouseNodes nodes;
@@ -46,6 +42,14 @@ public ClickHouseCredentials getDefaultCredentials() {
4642
return this.credentials;
4743
}
4844

45+
/**
46+
* Gets selected server.
47+
*
48+
* @return non-null selected server
49+
* @deprecated will be removed in v0.3.3, please use {@link #getNodes()}
50+
* instead
51+
*/
52+
@Deprecated
4953
public ClickHouseNode getServer() {
5054
return nodes.apply(nodes.getNodeSelector());
5155
}
@@ -54,6 +58,15 @@ public JdbcConfig getJdbcConfig() {
5458
return jdbcConf;
5559
}
5660

61+
/**
62+
* Gets nodes defined in connection string.
63+
*
64+
* @return non-null nodes
65+
*/
66+
public ClickHouseNodes getNodes() {
67+
return nodes;
68+
}
69+
5770
public Properties getProperties() {
5871
return props;
5972
}

0 commit comments

Comments
 (0)