Skip to content

Commit 259318d

Browse files
authored
Merge pull request #2852 from ClickHouse/05/14/26/fix_cloud_test_slowness
[repo] Fix slow init/teardown in cloud tests
2 parents 9cff77a + 6486bd9 commit 259318d

3 files changed

Lines changed: 65 additions & 16 deletions

File tree

.github/workflows/build.yml

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,24 @@ jobs:
256256
EOF
257257
- name: Install Java client
258258
run: mvn --also-make --batch-mode --no-transfer-progress -DskipTests=true -Dmaven.javadoc.skip=true install
259+
- name: Generate Database Name
260+
run: |
261+
RANDOM_DB="clickhouse_java_test_${GITHUB_RUN_ID}_${RANDOM}"
262+
echo "TEST_DB_NAME=$RANDOM_DB" >> $GITHUB_ENV
263+
echo "Generated database name: $RANDOM_DB"
264+
- name: Create Temporary Database
265+
env:
266+
CLICKHOUSE_CLOUD_HOST: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_HOST_SMT_PROD }}
267+
CLICKHOUSE_CLOUD_PASSWORD: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_PASSWORD_SMT_PROD }}
268+
run: |
269+
echo "Creating database: \`$TEST_DB_NAME\`"
270+
curl --fail-with-body -s -u "default:$CLICKHOUSE_CLOUD_PASSWORD" "https://$CLICKHOUSE_CLOUD_HOST:8443" --data-binary "CREATE DATABASE IF NOT EXISTS $TEST_DB_NAME"
259271
- name: Test http client
260272
env:
261-
CLICKHOUSE_CLOUD_HOST: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_HOST_SMT }}
262-
CLICKHOUSE_CLOUD_PASSWORD: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_PASSWORD_SMT }}
273+
CLICKHOUSE_CLOUD_HOST: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_HOST_SMT_PROD }}
274+
CLICKHOUSE_CLOUD_PASSWORD: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_PASSWORD_SMT_PROD }}
263275
CLIENT_JWT: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_JWT_DESERT_VM_43 }}
276+
JWT_TEST_HOST: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_HOST_SMT }}
264277
run: |
265278
mvn --batch-mode --no-transfer-progress --projects ${{ matrix.project }} -DclickhouseVersion=${{ matrix.clickhouse }} -Dprotocol=http -Dmaven.javadoc.skip=true verify
266279
- name: Upload test results
@@ -271,6 +284,20 @@ jobs:
271284
path: |
272285
**/target/failsafe-reports
273286
**/target/surefire-reports
287+
- name: Cleanup Database Unconditionally
288+
if: always() # CRITICAL: This ensures the step runs regardless of previous success/failure
289+
env:
290+
CLICKHOUSE_CLOUD_HOST: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_HOST_SMT_PROD }}
291+
CLICKHOUSE_CLOUD_PASSWORD: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_PASSWORD_SMT_PROD }}
292+
run: |
293+
# Verify we actually generated a DB name to avoid dropping something accidentally
294+
if [ -n "$TEST_DB_NAME" ]; then
295+
DROP_STMT="DROP DATABASE IF EXISTS \`$TEST_DB_NAME\`"
296+
echo "Cleaning up database: \`$TEST_DB_NAME\` with statement '$DROP_STMT'";
297+
curl --fail-with-body -s -u "default:$CLICKHOUSE_CLOUD_PASSWORD" "https://$CLICKHOUSE_CLOUD_HOST:8443" --data-binary "$DROP_STMT";
298+
else
299+
echo "No database name was generated; skipping cleanup.";
300+
fi;
274301
275302
test-jdbc-driver:
276303
runs-on: ubuntu-latest
@@ -321,9 +348,6 @@ jobs:
321348
- name: Install Java client
322349
run: mvn --also-make --batch-mode --no-transfer-progress --projects clickhouse-http-client,client-v2 -DskipTests=true -Dmaven.javadoc.skip=true install
323350
- name: Test JDBC driver
324-
env:
325-
CLICKHOUSE_CLOUD_HOST: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_HOST_SMT }}
326-
CLICKHOUSE_CLOUD_PASSWORD: ${{ secrets.INTEGRATIONS_TEAM_TESTS_CLOUD_PASSWORD_SMT }}
327351
run: |
328352
mvn --batch-mode --no-transfer-progress --projects clickhouse-jdbc,jdbc-v2 -DclickhouseVersion=${{ matrix.clickhouse }} -Dprotocol=${{ matrix.protocol }} -Dmaven.javadoc.skip=true verify
329353
- name: Upload test results

clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseServerForTest.java

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,23 @@ public class ClickHouseServerForTest {
6262
private static boolean isCloud = false;
6363

6464
private static final String database;
65+
private static final boolean localDatabase;
6566

6667
static {
6768
properties = new Properties(System.getProperties());
68-
database = "clickhouse_java_" + UUID.randomUUID().toString().substring(0, 8) + "_test_" + System.currentTimeMillis();
69+
String externalDatabase = System.getenv("TEST_DB_NAME"); // see build.yaml workflow
70+
if (externalDatabase != null && !externalDatabase.trim().isEmpty()) {
71+
if (!externalDatabase.startsWith("clickhouse_java_test_")) {
72+
throw new RuntimeException("external database for tests should start with 'clickhouse_java_test_'");
73+
}
74+
localDatabase = false;
75+
database = externalDatabase;
76+
} else {
77+
localDatabase = true;
78+
database = "clickhouse_java_" + UUID.randomUUID().toString().substring(0, 8) + "_test_" + System.currentTimeMillis();
79+
}
80+
81+
LOGGER.info("Local database: {}", localDatabase);
6982

7083
String proxy = properties.getProperty("proxyAddress");
7184
if (proxy != null && !proxy.isEmpty()) { // use external proxy
@@ -320,10 +333,11 @@ public static boolean isCloud() {
320333
@BeforeSuite(groups = {"integration"})
321334
public static void beforeSuite() {
322335
if (isCloud) {
323-
if (!runQuery("CREATE DATABASE IF NOT EXISTS " + database)) {
324-
throw new RuntimeException("Failed to create database for testing.");
336+
if (localDatabase) {
337+
if (!runQuery("CREATE DATABASE IF NOT EXISTS " + database)) {
338+
throw new RuntimeException("Failed to create database for testing.");
339+
}
325340
}
326-
327341
return;
328342
}
329343

@@ -358,8 +372,10 @@ public static void afterSuite() {
358372
}
359373

360374
if (isCloud) {
361-
if (!runQuery("DROP DATABASE IF EXISTS `" + database + "`")) {
362-
LOGGER.warn("Failed to drop database for testing.");
375+
if (localDatabase) {
376+
if (!runQuery("DROP DATABASE IF EXISTS `" + database + "`")) {
377+
LOGGER.warn("Failed to drop database for testing.");
378+
}
363379
}
364380
}
365381
}
@@ -391,16 +407,17 @@ public static boolean runQuery(String sql) {
391407
try {
392408
URL serverURL = new URL(uri);
393409
LOGGER.info("sending request to {} (uri={})", serverURL, uri);
394-
410+
byte[] postData = sql.getBytes(StandardCharsets.UTF_8);
395411
for (int attempts = 0; attempts < 10; attempts++) {
396412
HttpURLConnection httpConn = (HttpURLConnection) serverURL.openConnection();
397413
try {
398414
httpConn.setRequestMethod("POST");
399415
httpConn.setDoOutput(true);
400416
httpConn.setRequestProperty("Authorization", "Basic " + Base64.getEncoder().encodeToString(("default:" + getPassword()).getBytes()));
417+
httpConn.setFixedLengthStreamingMode(postData.length);
401418

402419
try (OutputStream out = httpConn.getOutputStream()) {
403-
out.write(sql.getBytes(StandardCharsets.UTF_8));
420+
out.write(postData, 0, postData.length);
404421
out.flush();
405422
}
406423

client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,10 +1218,18 @@ public void testJWTWithCloud() throws Exception {
12181218
if (!isCloud()) {
12191219
return; // only for cloud
12201220
}
1221-
String jwt = System.getenv("CLIENT_JWT");
1222-
Assert.assertTrue(jwt != null && !jwt.trim().isEmpty(), "JWT is missing");
1221+
final String jwt = System.getenv("CLIENT_JWT");
1222+
final String host = System.getenv("JWT_TEST_HOST");
1223+
Assert.assertTrue(jwt != null && !jwt.trim().isEmpty(), "CLIENT_JWT is not set.");
1224+
Assert.assertTrue(host != null && !host.trim().isEmpty(), "JWT_TEST_HOST is not set");
12231225
Assert.assertFalse(jwt.contains("\n") || jwt.contains("-----"), "JWT should be single string ready for HTTP header");
1224-
try (Client client = newClient().useBearerTokenAuth(jwt).build()) {
1226+
try (Client client = new Client.Builder()
1227+
.addEndpoint(Protocol.HTTP, host, 8443, true)
1228+
.setUsername("default")
1229+
.compressClientRequest(false)
1230+
.setDefaultDatabase("default")
1231+
.serverSetting(ServerSettings.WAIT_END_OF_QUERY, "1")
1232+
.useBearerTokenAuth(jwt).build()) {
12251233
try {
12261234
List<GenericRecord> response = client.queryAll("SELECT user(), now()");
12271235
System.out.println("response: " + response.get(0).getString(1) + " time: " + response.get(0).getString(2));

0 commit comments

Comments
 (0)