Skip to content

ClickHouseCommand.CommandTimeout is silently ignored — no client-side or server-side (max_execution_time) timeout is applied #618

Description

@claude

Description

ClickHouseCommand.CommandTimeout (the ADO.NET DbCommand.CommandTimeout override) is a dead
auto-property. Setting it has no effect at all: the command neither aborts client-side nor
passes max_execution_time to the server, so a query set to time out after N seconds runs to
completion and returns a result.

  • ClickHouse.Driver/ADO/ClickHouseCommand.cs:50public override int CommandTimeout { get; set; },
    doc-commented "Not currently used by ClickHouse." The property is never read anywhere in the
    repo (grep for CommandTimeout outside Vendor/ returns only this declaration).
  • ClickHouse.Driver/ADO/ClickHouseCommand.cs:240BuildQueryOptions() populates QueryId,
    BearerToken, Database, Roles, CustomSettings, AcceptEncoding — but never
    MaxExecutionTime, even though QueryOptions.MaxExecutionTime exists and works
    (ClickHouse.Driver/QueryOptions.cs:98).

So the low-level IClickHouseClient API can bound server-side execution, but the ADO.NET surface —
which is what EF Core, Dapper, and any DbCommand-based consumer configures — cannot. ADO.NET
consumers reasonably assume CommandTimeout bounds the operation; here a runaway query keeps
burning server CPU and the caller keeps waiting indefinitely.

This mirrors the second half of ClickHouse/clickhouse-java#3136
(Statement.setQueryTimeout() not setting max_execution_time). Note that in clickhouse-java
setQueryTimeout at least bounds the client-side wait; in this driver CommandTimeout does
nothing whatsoever.

The first half of that Java issue (error 159 TIMEOUT_EXCEEDED being classified as retryable and
the query being silently re-executed) does not apply here — this driver has no automatic query
retry logic, so a 159 surfaces once as a ClickHouseServerException.

ClickHouse server version

26.8.6.5 (official build), reached over HTTP at localhost:8123.

Reproduction

NUnit test using the project's own fixture (ClickHouse.Driver.Tests):

using System;
using System.Diagnostics;
using System.Threading.Tasks;
using NUnit.Framework;

namespace ClickHouse.Driver.Tests;

public class CommandTimeoutTests : AbstractConnectionTestFixture
{
    private const string SlowQuery =
        "SELECT sum(sleepEachRow(1)) FROM numbers(4) SETTINGS function_sleep_max_microseconds_per_block = 10000000";

    [Test]
    public async Task CommandTimeoutShouldLimitServerSideExecution()
    {
        using var command = connection.CreateCommand();
        command.CommandText = SlowQuery;
        command.CommandTimeout = 1; // 1 second

        var sw = Stopwatch.StartNew();
        object result = null;
        Exception thrown = null;
        try { result = await command.ExecuteScalarAsync(); }
        catch (Exception e) { thrown = e; }
        sw.Stop();

        TestContext.Out.WriteLine($"elapsed={sw.Elapsed.TotalSeconds:F1}s result={result} exception={thrown?.GetType().Name}");

        Assert.That(sw.Elapsed.TotalSeconds, Is.LessThan(3.0),
            "CommandTimeout=1 did not limit execution; query ran to completion server-side");
    }

    // Control: the low-level API does honour the setting.
    [Test]
    public void MaxExecutionTimeViaQueryOptionsDoesWork()
    {
        var options = new QueryOptions { MaxExecutionTime = TimeSpan.FromSeconds(1) };
        var ex = Assert.ThrowsAsync<ClickHouseServerException>(
            async () => await connection.ClickHouseClient.ExecuteScalarAsync(SlowQuery, null, options));
        TestContext.Out.WriteLine($"control errorCode={ex.ErrorCode}");
    }
}

Expected (CommandTimeoutShouldLimitServerSideExecution): with CommandTimeout = 1 the call
fails fast — a ClickHouseServerException with ErrorCode = 159, or at minimum a client-side
timeout — well before the query's 4 s of work completes.

Actual: the test fails.

elapsed=4.0s result=0 exception=:
  Failed CommandTimeoutShouldLimitServerSideExecution [4 s]
  Error Message:
     CommandTimeout=1 did not limit execution; query ran to completion server-side
  Expected: less than 3.0d
  But was:  4.0116792999999999d

The control test passes, confirming the plumbing exists one layer down:

control errorCode=159
Code: 159. DB::Exception: Timeout exceeded: elapsed 1031.811 ms, maximum: 1000.000 ms.
  (TIMEOUT_EXCEEDED) (version 26.8.6.5 (official build))

Suggested fix

In ClickHouse.Driver/ADO/ClickHouseCommand.cs, BuildQueryOptions() (line 240) should map a
non-zero CommandTimeout onto MaxExecutionTime, e.g.:

MaxExecutionTime = CommandTimeout > 0 ? TimeSpan.FromSeconds(CommandTimeout) : null,

Points worth deciding as part of the fix:

  • CommandTimeout defaults to 0 here (the auto-property's default), whereas ADO.NET convention is
    30 s with 0 meaning "no limit". Whichever is chosen, 0 should keep meaning no limit so existing
    users who never touched the property see no behaviour change.
  • Consider also bounding the client-side wait (a CancellationTokenSource linked into lcts in the
    Execute*Async methods), so the caller is released even if the server is unresponsive rather than
    merely slow.
  • An explicit QueryOptions.MaxExecutionTime passed through the low-level API should keep taking
    precedence.
  • The XML doc on line 48–49 ("Not currently used by ClickHouse") needs updating.

Link

Relayed from ClickHouse/clickhouse-java#3136

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions