jdbc-v2: session_id not accepted as a connection property, and cancel() doesn't strip a custom_http_params session_id → SESSION_IS_LOCKED

[vprotasov]

When a long-running query holds that HTTP session, StatementImpl.cancel() issues KILL QUERY over the same connection. Because the session id is carried in custom_http_params, the KILL becomes a second concurrent request in the locked session and the server returns 373 SESSION_IS_LOCKED. The query is never cancelled.

PR #2868 added clearSession() and strips session_id/session_check/session_timeout from the internally issued KILL QUERY — but it does not touch custom_http_params, so a session id provided that way is still sent with the KILL and the cancel still fails.

Steps to reproduce
▎ 1. Connect with the v2 driver, setting custom_http_params=session_id=my_session.
▎ 2. Start a long-running query (e.g. SELECT sleep(3) FROM numbers(100)).
▎ 3. From the same Statement, call cancel().
▎ 4. Observe KILL QUERY fails with Code: 373. DB::Exception: Session ... is locked by a concurrent client; the query runs to completion.

Requests (either or both)
▎ 1. Preferred: accept session_id (and session_check/session_timeout) as first-class jdbc-v2 connection properties, so clients don't have to use custom_http_params for sessions at all.
▎ 2. Have cancel() also detect and strip a session_id carried in custom_http_params from the internally issued KILL QUERY (extending #2868), so it isn't blocked by the session lock.

Option 1 also lets the existing clearSession() logic from #2868 handle cancel correctly without any client-side workaround.

related:
#871

[chernser]

@vprotasov
thank you for reporting the issue!
Will fix it.

[chernser]

/triage

[github-actions]

Triage Report

Effort to Fix: Small
Type: Bug
Affected Module: jdbc-v2

Summary

When session_id is supplied via custom_http_params (a deprecated but still-supported JDBC v2 connection property), JdbcConfiguration stores it as a client-level property rather than in the statement's QuerySettings. This means StatementImpl.cancel() calls clearSession() on QuerySettings but that call is a no-op for a session_id that lives at the client level — so the KILL QUERY is sent with the locked session_id, and the server returns error 373 (SESSION_IS_LOCKED), leaving the query uncancelled. A second request is to accept session_id/session_check/session_timeout as first-class JDBC connection properties so clients don't need custom_http_params at all; this would also make the existing clearSession() path work correctly.

Recommended Labels & Priority

• Labels: bug, jdbc-v2, area:network

Missing Information / Questions for User

1. Which version of the jdbc-v2 driver is being used? (The fix in PR [jdbc-v2] Fix Statement.cancel() for query run with session_id parameters  #2868 was merged recently; confirm the reported behaviour exists on the latest snapshot or a specific release.)
2. Is session_id being passed via the JDBC URL query string (?custom_http_params=session_id=...), via Properties, or both? (Helps narrow the JdbcConfiguration parsing path that needs patching.)
3. Is the ClickHouse server version relevant? (SESSION_IS_LOCKED is server-side; confirming the server version rules out any server-side behaviour changes.)

Tests to Add

Test 1: cancel() succeeds when session_id is set via custom_http_params

// Connect with custom_http_params carrying a session_id, run a slow query,
// call cancel() from another thread, and assert the query does not complete
// (i.e. no SESSION_IS_LOCKED exception is thrown and the statement is cancelled).
@Test
void testCancelWithSessionIdInCustomHttpParams() throws Exception {
    Properties props = new Properties();
    props.setProperty("custom_http_params", "session_id=test_session_cancel");
    try (Connection conn = DriverManager.getConnection(jdbcUrl, props);
         Statement stmt = conn.createStatement()) {

        Future<?> query = executor.submit(() -> {
            try {
                stmt.execute("SELECT sleep(10) FROM numbers(100)");
            } catch (SQLException e) {
                // expected — query should be killed
            }
        });

        Thread.sleep(500); // let the query start
        // Must not throw SESSION_IS_LOCKED (code 373)
        assertDoesNotThrow(stmt::cancel);
        query.get(5, TimeUnit.SECONDS);
    }
}

Test 2: session_id accepted as a first-class JDBC connection property

// Verify that session_id can be supplied directly as a connection property
// (not buried in custom_http_params) and that cancel() still works correctly.
@Test
void testCancelWithFirstClassSessionIdProperty() throws Exception {
    Properties props = new Properties();
    props.setProperty("session_id", "test_session_firstclass");
    try (Connection conn = DriverManager.getConnection(jdbcUrl, props);
         Statement stmt = conn.createStatement()) {

        Future<?> query = executor.submit(() -> {
            try {
                stmt.execute("SELECT sleep(10) FROM numbers(100)");
            } catch (SQLException e) {
                // expected — query should be killed
            }
        });

        Thread.sleep(500);
        assertDoesNotThrow(stmt::cancel);
        query.get(5, TimeUnit.SECONDS);
    }
}

[vprotasov]

Missing Information / Questions for User

Which version of the jdbc-v2 driver is being used?
0.9.8
Is session_id being passed via the JDBC URL query string (?custom_http_params=session_id=...), via Properties, or both?
via Properties only for the auto-injected session id.

Is the ClickHouse server version relevant?
not sure, reproduce with ClickHouse 26.5.1.882

reproducible with the latest DataGrip EAP build

🔗 https://www.jetbrains.com/datagrip/nextversion/

[chernser]

Good day, @vprotasov !

Would you please review #2885 ?

We will consider adding session_id and others to DriverSettings.

[vprotasov]

Thanks for the fix!
Reviewing for our DataGrip use case (cancel of a long-running query over a ClickHouse HTTP session), I think this PR resolves #2881 only for clients that set session_id as a first-class property, but not for clients that inject it via custom_http_params.

In StatementImpl.cancel(), clearSession() only nulls the session_id/session_check/session_timeout server keys. It doesn't touch custom_http_params, so a session carried as custom_http_params=session_id= is still serialized on the KILL QUERY → it runs as a second concurrent request in the locked session → server returns 373 SESSION_IS_LOCKED, same as before.

This is exactly how DataGrip injects the session (each console connection gets a session_id via custom_http_params), so cancel still fails for us after this change.

Could you pleae also implement Option 2 from #2881 — strip a session_id (and related session keys) out of custom_http_params when issuing the internal KILL? That keeps the existing custom_http_params mechanism working without requiring every client to migrate to the new first-class property.

A test covering the custom_http_params-supplied-session cancel path would also help, since the current tests only exercise the first-class-property API.

[chernser]

@vprotasov
Thank you for review!
I will check the case with custom http params (forgot to make it because it is deprecated way) but idea of the fix to place null as setting value to make http transport send nothing. custom http params are only part of jdbc so client should see unwrapped settings (it is done in JdbcConfiguration)