clickhouse-java icon indicating copy to clipboard operation
clickhouse-java copied to clipboard

Inconsistent Use of executionTimeout Parameter in Client Component

Open KciNKh opened this issue 9 months ago • 0 comments

Description

In the Client component, the executionTimeout parameter is used with different time units, leading to confusion during its usage.

  • When setting the value: The executionTimeout parameter is interpreted in milliseconds.
  • When retrieving the value: The parameter is returned in seconds.

Methods with Inconsistent Usage:

  1. setExecutionTimeout(long timeout, ChronoUnit timeUnit):

    • This method sets the executionTimeout in milliseconds.
    public Builder setExecutionTimeout(long timeout, ChronoUnit timeUnit) {
        this.configuration.put(ClientConfigProperties.MAX_EXECUTION_TIME.getKey(), String.valueOf(Duration.of(timeout, timeUnit).toMillis()));
        return this;
    }
    
  2. getOperationTimeout():

    • This method retrieves the executionTimeout but does not clarify the unit of measurement in its JavaDoc, leading to confusion as it returns the value as an integer.
    protected int getOperationTimeout() {
        return Integer.parseInt((String)this.configuration.get(ClientConfigProperties.MAX_EXECUTION_TIME.getKey()));
    }
    
  3. Usage in queryAll and getTableSchemaImpl:

    • In queryAll, the timeout is used in milliseconds:
    try (QueryResponse response = operationTimeout == 0 ? query(sqlQuery, settings).get() :
            query(sqlQuery, settings).get(operationTimeout, TimeUnit.MILLISECONDS)) {
        ...
    }
    
    • In getTableSchemaImpl, the timeout is used in seconds:
    try (QueryResponse response = operationTimeout == 0
            ? query(describeQuery, queryParams, settings).get()
            : query(describeQuery, queryParams, settings).get(getOperationTimeout(), TimeUnit.SECONDS)) {
        ...
    }
    

JavaDoc Issue: The JavaDoc for this method states that the returned value is in seconds, which is inconsistent with how the value is set.

Expected Behavior:

The units of measurement should be consistent. It is recommended to use the same unit of measurement (either milliseconds or seconds) for both setting and retrieving the value. Additionally, the JavaDoc for the getOperationTimeout() method should be updated to reflect the correct unit of measurement.

Steps to Reproduce:

  1. Set the executionTimeout value in the Client component (e.g., 1000).
  2. Retrieve the executionTimeout value.
  3. Note the discrepancy in units of measurement.

Additional Information: This inconsistency may lead to errors and misunderstandings on the part of users of the component. The JavaDoc for the method that retrieves this value should be corrected to indicate the correct unit of measurement.

Environment

  • Client-v2 version: 0.8.5

KciNKh avatar May 10 '25 08:05 KciNKh