From 9f6a6a2b0d8e3cf4e515aa414c810e9d46efa4f9 Mon Sep 17 00:00:00 2001 From: Tanmaya Panda Date: Tue, 28 Apr 2026 23:42:00 +0530 Subject: [PATCH] Fix double-application of 30s client grace period (v8.0.1) PR #426 (v7.0.6) added a 30-second client grace period in ClientImpl.determineTimeout. However, BaseClient.getContextTimeout was already adding the same 30-second buffer (EXTRA_TIMEOUT_FOR_CLIENT_SIDE) when constructing the HTTP request context, so every request used an effective HTTP-client timeout of serverTimeout + 60s instead of the intended serverTimeout + 30s. Removes the redundant grace from determineTimeout. The single 30s grace in BaseClient.getContextTimeout is preserved (pre-7.0.6 behavior). The servertimeout header sent to the service is unchanged. Adds ClientImplTimeoutTest pinning determineTimeout's return to the raw timeout (no grace at this layer) so the regression cannot be silently reintroduced. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 8 ++ .../azure/kusto/data/ClientImpl.java | 8 +- .../kusto/data/ClientImplTimeoutTest.java | 95 +++++++++++++++++++ pom.xml | 2 +- quickstart/pom.xml | 2 +- 5 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 data/src/test/java/com/microsoft/azure/kusto/data/ClientImplTimeoutTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index ef5d3c8a8..fc04fe19e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [8.0.1] - 2026-04-28 + +### Fixed +- Fixed double-application of the 30-second client grace period on every request. + In v7.0.6 a 30s grace was added in `ClientImpl.determineTimeout`, but `BaseClient.getContextTimeout` + was already adding the same 30s buffer (`EXTRA_TIMEOUT_FOR_CLIENT_SIDE`). The effective HTTP-client + timeout was `serverTimeout + 60s`; it is now restored to `serverTimeout + 30s`. + ## [8.0.0] - 2026-02-04 ### Changed diff --git a/data/src/main/java/com/microsoft/azure/kusto/data/ClientImpl.java b/data/src/main/java/com/microsoft/azure/kusto/data/ClientImpl.java index 8b1ee2145..ff3357ec2 100644 --- a/data/src/main/java/com/microsoft/azure/kusto/data/ClientImpl.java +++ b/data/src/main/java/com/microsoft/azure/kusto/data/ClientImpl.java @@ -11,7 +11,6 @@ import java.util.Map; import java.util.concurrent.TimeUnit; - import org.jetbrains.annotations.NotNull; import com.azure.core.http.HttpClient; @@ -45,7 +44,6 @@ class ClientImpl extends BaseClient { public static final String MGMT_ENDPOINT_VERSION = "v1"; public static final String QUERY_ENDPOINT_VERSION = "v2"; public static final String STREAMING_VERSION = "v1"; - private static final Long CLIENT_GRACE_PERIOD_IN_MILLISECS = TimeUnit.SECONDS.toMillis(30); private static final Long COMMAND_TIMEOUT_IN_MILLISECS = TimeUnit.MINUTES.toMillis(10); private static final Long QUERY_TIMEOUT_IN_MILLISECS = TimeUnit.MINUTES.toMillis(4); private static final Long STREAMING_INGEST_TIMEOUT_IN_MILLISECS = TimeUnit.MINUTES.toMillis(10); @@ -412,7 +410,9 @@ private Mono executeStreamingQuery(String clusterEndpoint, KustoReq "ClientImpl.executeStreamingQuery", updateAndGetExecuteTracingAttributes(kr.getDatabase(), properties))); } - private long determineTimeout(ClientRequestProperties properties, CommandType commandType, String clusterUrl) { + // Package-private for testability. The 30s client-side grace is applied once in BaseClient.getContextTimeout + // (via EXTRA_TIMEOUT_FOR_CLIENT_SIDE); do not add it here as well or it will be applied twice. + long determineTimeout(ClientRequestProperties properties, CommandType commandType, String clusterUrl) { Object skipBoolean = properties.getOption(ClientRequestProperties.OPTION_NO_REQUEST_TIMEOUT); if (skipBoolean instanceof Boolean && (Boolean) skipBoolean) { return Long.MAX_VALUE; @@ -440,7 +440,7 @@ private long determineTimeout(ClientRequestProperties properties, CommandType co // If we set the timeout ourself, we need to update the server header properties.setTimeoutInMilliSec(timeoutMs); - return timeoutMs + CLIENT_GRACE_PERIOD_IN_MILLISECS; + return timeoutMs; } private Mono getAuthorizationHeaderValueAsync() { diff --git a/data/src/test/java/com/microsoft/azure/kusto/data/ClientImplTimeoutTest.java b/data/src/test/java/com/microsoft/azure/kusto/data/ClientImplTimeoutTest.java new file mode 100644 index 000000000..d455e0a75 --- /dev/null +++ b/data/src/test/java/com/microsoft/azure/kusto/data/ClientImplTimeoutTest.java @@ -0,0 +1,95 @@ +package com.microsoft.azure.kusto.data; + +import com.microsoft.azure.kusto.data.auth.ConnectionStringBuilder; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.net.URISyntaxException; +import java.util.concurrent.TimeUnit; + +/** + * Regression tests for the v7.0.6 "double grace period" bug. + *

+ * In v7.0.6, {@code ClientImpl.determineTimeout} added a 30-second client grace period to + * its return value. However, {@code BaseClient.getContextTimeout} already adds the same + * 30-second buffer ({@code EXTRA_TIMEOUT_FOR_CLIENT_SIDE}) when constructing the HTTP + * request context. The result was that every request used an effective client-side + * timeout of {@code serverTimeout + 60s} instead of {@code serverTimeout + 30s}. + *

+ * The fix removes the grace from {@code determineTimeout}; this test pins + * {@code determineTimeout} to return the raw timeout (no grace) so the regression cannot + * be reintroduced silently. + */ +public class ClientImplTimeoutTest { + + private static final long QUERY_DEFAULT_MS = TimeUnit.MINUTES.toMillis(4); + private static final long ADMIN_DEFAULT_MS = TimeUnit.MINUTES.toMillis(10); + private static final long STREAMING_INGEST_DEFAULT_MS = TimeUnit.MINUTES.toMillis(10); + + private ClientImpl newClient() throws URISyntaxException { + ConnectionStringBuilder csb = ConnectionStringBuilder.createWithAadManagedIdentity("https://testcluster.kusto.windows.net"); + return (ClientImpl) ClientFactory.createClient(csb); + } + + @Test + @DisplayName("determineTimeout returns default query timeout (no grace added at this layer)") + void defaultQueryTimeout() throws URISyntaxException { + ClientImpl client = newClient(); + ClientRequestProperties crp = new ClientRequestProperties(); + long actual = client.determineTimeout(crp, CommandType.QUERY, "https://testcluster.kusto.windows.net"); + Assertions.assertEquals(QUERY_DEFAULT_MS, actual); + } + + @Test + @DisplayName("determineTimeout returns default admin command timeout (no grace added at this layer)") + void defaultAdminTimeout() throws URISyntaxException { + ClientImpl client = newClient(); + ClientRequestProperties crp = new ClientRequestProperties(); + long actual = client.determineTimeout(crp, CommandType.ADMIN_COMMAND, "https://testcluster.kusto.windows.net"); + Assertions.assertEquals(ADMIN_DEFAULT_MS, actual); + } + + @Test + @DisplayName("determineTimeout returns default streaming ingest timeout (no grace added at this layer)") + void defaultStreamingIngestTimeout() throws URISyntaxException { + ClientImpl client = newClient(); + ClientRequestProperties crp = new ClientRequestProperties(); + long actual = client.determineTimeout(crp, CommandType.STREAMING_INGEST, "https://testcluster.kusto.windows.net"); + Assertions.assertEquals(STREAMING_INGEST_DEFAULT_MS, actual); + } + + @Test + @DisplayName("determineTimeout returns user-provided timeout verbatim (no grace added at this layer)") + void userProvidedTimeout() throws URISyntaxException { + ClientImpl client = newClient(); + ClientRequestProperties crp = new ClientRequestProperties(); + long userTimeout = TimeUnit.MINUTES.toMillis(7); + crp.setTimeoutInMilliSec(userTimeout); + + long actual = client.determineTimeout(crp, CommandType.QUERY, "https://testcluster.kusto.windows.net"); + Assertions.assertEquals(userTimeout, actual); + } + + @Test + @DisplayName("determineTimeout returns Long.MAX_VALUE when OPTION_NO_REQUEST_TIMEOUT is true") + void noRequestTimeoutOption() throws URISyntaxException { + ClientImpl client = newClient(); + ClientRequestProperties crp = new ClientRequestProperties(); + crp.setOption(ClientRequestProperties.OPTION_NO_REQUEST_TIMEOUT, true); + + long actual = client.determineTimeout(crp, CommandType.QUERY, "https://testcluster.kusto.windows.net"); + Assertions.assertEquals(Long.MAX_VALUE, actual); + } + + @Test + @DisplayName("determineTimeout writes the raw (no-grace) timeout back into the server-timeout option") + void serverTimeoutHeaderUnaffectedByGrace() throws URISyntaxException { + ClientImpl client = newClient(); + ClientRequestProperties crp = new ClientRequestProperties(); + client.determineTimeout(crp, CommandType.QUERY, "https://testcluster.kusto.windows.net"); + + // Server-timeout option must reflect the raw default (4 minutes), not a graced value. + Assertions.assertEquals("00:04:00", crp.getOption(ClientRequestProperties.OPTION_SERVER_TIMEOUT)); + } +} diff --git a/pom.xml b/pom.xml index b493f99cb..704ddba37 100644 --- a/pom.xml +++ b/pom.xml @@ -32,7 +32,7 @@ - 8.0.0 + 8.0.1 0.0.1-beta UTF-8 11 1.2.28 diff --git a/quickstart/pom.xml b/quickstart/pom.xml index 0baa73b4d..4f4dda7c2 100644 --- a/quickstart/pom.xml +++ b/quickstart/pom.xml @@ -33,7 +33,7 @@ - 8.0.0 + 8.0.1 11 3.2.0 3.8.1