more changes

jtuglu1 · jtuglu1 · commit a18d77f66452 · 2026-05-27T15:35:09.000-07:00
diff --git a/extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/KubernetesTaskExecutionConfigResource.java b/extensions-core/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/execution/KubernetesTaskExecutionConfigResource.java
@@ -161,10 +161,14 @@ public Response getExecutionConfigHistory(
   @ResourceFilters(ConfigResourceFilter.class)
   public Response getExecutionConfig()
   {
-    return DynamicConfigEtagHelper.withEtag(
-        Response.ok(getDynamicConfig()),
-        configManager.getCurrentBytes(KubernetesTaskRunnerDynamicConfig.CONFIG_KEY)
-    ).build();
+    return DynamicConfigEtagHelper.buildReadResponseWithEtag(
+        () -> configManager.getCurrentBytes(KubernetesTaskRunnerDynamicConfig.CONFIG_KEY),
+        currentBytes -> configManager.convertByteToConfig(
+            currentBytes,
+            KubernetesTaskRunnerDynamicConfig.class,
+            null
+        )
+    );
   }
 
   private KubernetesTaskRunnerDynamicConfig getDynamicConfig()
diff --git a/extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/KubernetesTaskExecutionConfigResourceTest.java b/extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/execution/KubernetesTaskExecutionConfigResourceTest.java
@@ -20,6 +20,7 @@
 package org.apache.druid.k8s.overlord.execution;
 
 import org.apache.druid.audit.AuditManager;
+import org.apache.druid.common.config.ConfigEtag;
 import org.apache.druid.common.config.ConfigManager;
 import org.apache.druid.common.config.JacksonConfigManager;
 import org.apache.druid.error.ErrorResponse;
@@ -33,6 +34,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response;
+import java.nio.charset.StandardCharsets;
 import java.util.concurrent.atomic.AtomicReference;
 
 public class KubernetesTaskExecutionConfigResourceTest
@@ -51,6 +53,36 @@ public void setUp()
     dynamicConfig = EasyMock.createMock(KubernetesTaskRunnerDynamicConfig.class);
   }
 
+  @Test
+  public void getExecutionConfigDerivesBodyAndEtagFromSameBytes()
+  {
+    final byte[] currentBytes = "current-k8s-execution-config".getBytes(StandardCharsets.UTF_8);
+    final KubernetesTaskRunnerDynamicConfig expectedConfig =
+        new DefaultKubernetesTaskRunnerDynamicConfig(new TaskTypePodTemplateSelectStrategy(), 10);
+    final KubernetesTaskExecutionConfigResource testedResource = new KubernetesTaskExecutionConfigResource(
+        configManager,
+        auditManager
+    );
+
+    EasyMock.expect(configManager.getCurrentBytes(KubernetesTaskRunnerDynamicConfig.CONFIG_KEY))
+            .andReturn(currentBytes)
+            .once();
+    EasyMock.expect(
+        configManager.convertByteToConfig(
+            EasyMock.aryEq(currentBytes),
+            EasyMock.eq(KubernetesTaskRunnerDynamicConfig.class),
+            (KubernetesTaskRunnerDynamicConfig) EasyMock.isNull()
+        )
+    ).andReturn(expectedConfig).once();
+    EasyMock.replay(configManager, auditManager);
+
+    final Response result = testedResource.getExecutionConfig();
+
+    Assertions.assertEquals(Response.Status.OK.getStatusCode(), result.getStatus());
+    Assertions.assertEquals(expectedConfig, result.getEntity());
+    Assertions.assertEquals(ConfigEtag.compute(currentBytes), result.getMetadata().getFirst(HttpHeaders.ETAG));
+  }
+
   @Test
   public void setExecutionConfigSuccessfulUpdate()
   {
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordCompactionResource.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordCompactionResource.java
@@ -138,16 +138,10 @@ public Response updateClusterCompactionConfig(
   @ResourceFilters(ConfigResourceFilter.class)
   public Response getClusterCompactionConfig()
   {
-    final Response base = ServletResourceUtils.buildReadResponse(
-        configManager::getClusterCompactionConfig
+    return DynamicConfigEtagHelper.buildReadResponseWithEtag(
+        configManager::getCurrentCompactionConfigBytes,
+        currentBytes -> configManager.convertBytesToCompactionConfig(currentBytes).clusterConfig()
     );
-    if (base.getStatus() != Response.Status.OK.getStatusCode()) {
-      return base;
-    }
-    return DynamicConfigEtagHelper.withEtag(
-        Response.fromResponse(base),
-        configManager.getCurrentCompactionConfigBytes()
-    ).build();
   }
 
   @GET
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java
@@ -427,10 +427,14 @@ public Response getMultipleTaskStatuses(Set<String> taskIds)
   @ResourceFilters(ConfigResourceFilter.class)
   public Response getWorkerConfig()
   {
-    return DynamicConfigEtagHelper.withEtag(
-        Response.ok(taskQueryTool.getLatestWorkerConfig()),
-        configManager.getCurrentBytes(WorkerBehaviorConfig.CONFIG_KEY)
-    ).build();
+    return DynamicConfigEtagHelper.buildReadResponseWithEtag(
+        () -> configManager.getCurrentBytes(WorkerBehaviorConfig.CONFIG_KEY),
+        currentBytes -> configManager.convertByteToConfig(
+            currentBytes,
+            WorkerBehaviorConfig.class,
+            null
+        )
+    );
   }
 
   /**
diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordCompactionResourceTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordCompactionResourceTest.java
@@ -23,6 +23,7 @@
 import com.google.common.util.concurrent.Futures;
 import org.apache.druid.audit.AuditManager;
 import org.apache.druid.client.coordinator.CoordinatorClient;
+import org.apache.druid.common.config.ConfigEtag;
 import org.apache.druid.error.DruidExceptionMatcher;
 import org.apache.druid.error.ErrorResponse;
 import org.apache.druid.indexer.CompactionEngine;
@@ -41,6 +42,7 @@
 import org.apache.druid.server.coordinator.CoordinatorConfigManager;
 import org.apache.druid.server.coordinator.DataSourceCompactionConfig;
 import org.apache.druid.server.coordinator.DataSourceCompactionConfigAuditEntry;
+import org.apache.druid.server.coordinator.DruidCompactionConfig;
 import org.apache.druid.server.coordinator.InlineSchemaDataSourceCompactionConfig;
 import org.apache.druid.server.security.AllowAllAuthorizer;
 import org.apache.druid.server.security.AuthConfig;
@@ -57,6 +59,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
@@ -192,17 +195,20 @@ public void test_getClusterConfig()
   {
     final ClusterCompactionConfig clusterConfig =
         new ClusterCompactionConfig(0.4, 100, null, true, CompactionEngine.MSQ, true);
-    EasyMock.expect(configManager.getClusterCompactionConfig())
-            .andReturn(clusterConfig)
-            .once();
+    final DruidCompactionConfig compactionConfig = DruidCompactionConfig.empty().withClusterConfig(clusterConfig);
+    final byte[] currentBytes = "current-compaction-config".getBytes(StandardCharsets.UTF_8);
     EasyMock.expect(configManager.getCurrentCompactionConfigBytes())
-            .andReturn(null)
+            .andReturn(currentBytes)
+            .once();
+    EasyMock.expect(configManager.convertBytesToCompactionConfig(EasyMock.aryEq(currentBytes)))
+            .andReturn(compactionConfig)
             .once();
     replayAll();
 
     final Response response = compactionResource.getClusterCompactionConfig();
     Assert.assertEquals(200, response.getStatus());
     Assert.assertEquals(clusterConfig, response.getEntity());
+    Assert.assertEquals(ConfigEtag.compute(currentBytes), response.getMetadata().getFirst(HttpHeaders.ETAG));
   }
 
   @Test
diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java
@@ -27,6 +27,7 @@
 import com.google.common.collect.ImmutableSet;
 import org.apache.druid.audit.AuditEntry;
 import org.apache.druid.audit.AuditManager;
+import org.apache.druid.common.config.ConfigEtag;
 import org.apache.druid.common.config.JacksonConfigManager;
 import org.apache.druid.error.DruidException;
 import org.apache.druid.error.ErrorResponse;
@@ -94,6 +95,7 @@
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.Response.Status;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -1365,6 +1367,31 @@ public void testGetTotalWorkerCapacityWithMaximumCapacity()
     Assert.assertEquals(expectedWorkerCapacityWithAutoscale, ((TotalWorkerCapacityResponse) response.getEntity()).getMaximumCapacityWithAutoScale());
   }
 
+  @Test
+  public void testGetWorkerConfigDerivesBodyAndEtagFromSameBytes()
+  {
+    final WorkerBehaviorConfig workerBehaviorConfig = EasyMock.createMock(WorkerBehaviorConfig.class);
+    final byte[] currentBytes = "current-worker-config".getBytes(StandardCharsets.UTF_8);
+
+    EasyMock.expect(configManager.getCurrentBytes(WorkerBehaviorConfig.CONFIG_KEY))
+            .andReturn(currentBytes)
+            .once();
+    EasyMock.expect(
+        configManager.convertByteToConfig(
+            EasyMock.aryEq(currentBytes),
+            EasyMock.eq(WorkerBehaviorConfig.class),
+            (WorkerBehaviorConfig) EasyMock.isNull()
+        )
+    ).andReturn(workerBehaviorConfig).once();
+    replayAll();
+
+    final Response response = overlordResource.getWorkerConfig();
+
+    Assert.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
+    Assert.assertEquals(workerBehaviorConfig, response.getEntity());
+    Assert.assertEquals(ConfigEtag.compute(currentBytes), response.getMetadata().getFirst(HttpHeaders.ETAG));
+  }
+
   @Test
   public void testSetWorkerConfigWithBlankIfMatchReturnsBadRequest()
   {
diff --git a/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorConfigManager.java b/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorConfigManager.java
@@ -88,6 +88,15 @@ public CoordinatorDynamicConfig getCurrentDynamicConfig()
     return Preconditions.checkNotNull(dynamicConfig, "Got null config from watcher?!");
   }
 
+  public CoordinatorDynamicConfig convertBytesToDynamicConfig(@Nullable byte[] bytes)
+  {
+    return jacksonConfigManager.convertByteToConfig(
+        bytes,
+        CoordinatorDynamicConfig.class,
+        CoordinatorDynamicConfig.builder().build()
+    );
+  }
+
   public ConfigManager.SetResult setDynamicConfig(CoordinatorDynamicConfig config, AuditInfo auditInfo)
   {
     return setDynamicConfig(config, null, auditInfo);
@@ -190,7 +199,7 @@ public ConfigManager.SetResult getAndUpdateCompactionConfig(
     }
   }
 
-  public DruidCompactionConfig convertBytesToCompactionConfig(byte[] bytes)
+  public DruidCompactionConfig convertBytesToCompactionConfig(@Nullable byte[] bytes)
   {
     return jacksonConfigManager.convertByteToConfig(
         bytes,
diff --git a/server/src/main/java/org/apache/druid/server/http/CoordinatorBrokerConfigsResource.java b/server/src/main/java/org/apache/druid/server/http/CoordinatorBrokerConfigsResource.java
@@ -76,10 +76,14 @@ public CoordinatorBrokerConfigsResource(
   @Produces(MediaType.APPLICATION_JSON)
   public Response getBrokerDynamicConfig()
   {
-    return DynamicConfigEtagHelper.withEtag(
-        Response.ok(currentConfig.get()),
-        configManager.getCurrentBytes(BrokerDynamicConfig.CONFIG_KEY)
-    ).build();
+    return DynamicConfigEtagHelper.buildReadResponseWithEtag(
+        () -> configManager.getCurrentBytes(BrokerDynamicConfig.CONFIG_KEY),
+        currentBytes -> configManager.convertByteToConfig(
+            currentBytes,
+            BrokerDynamicConfig.class,
+            BrokerDynamicConfig.builder().build()
+        )
+    );
   }
 
   @POST
diff --git a/server/src/main/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResource.java b/server/src/main/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResource.java
@@ -60,16 +60,10 @@ public CoordinatorCompactionConfigsResource(
   @Produces(MediaType.APPLICATION_JSON)
   public Response getCompactionConfig()
   {
-    final Response baseResponse = ServletResourceUtils.buildReadResponse(
-        configManager::getCurrentCompactionConfig
+    return DynamicConfigEtagHelper.buildReadResponseWithEtag(
+        configManager::getCurrentCompactionConfigBytes,
+        configManager::convertBytesToCompactionConfig
     );
-    if (baseResponse.getStatus() != Response.Status.OK.getStatusCode()) {
-      return baseResponse;
-    }
-    return DynamicConfigEtagHelper.withEtag(
-        Response.fromResponse(baseResponse),
-        configManager.getCurrentCompactionConfigBytes()
-    ).build();
   }
 
   /**
diff --git a/server/src/main/java/org/apache/druid/server/http/CoordinatorDynamicConfigsResource.java b/server/src/main/java/org/apache/druid/server/http/CoordinatorDynamicConfigsResource.java
@@ -75,10 +75,10 @@ public CoordinatorDynamicConfigsResource(
   @Produces(MediaType.APPLICATION_JSON)
   public Response getDynamicConfigs()
   {
-    return DynamicConfigEtagHelper.withEtag(
-        Response.ok(manager.getCurrentDynamicConfig()),
-        manager.getCurrentDynamicConfigBytes()
-    ).build();
+    return DynamicConfigEtagHelper.buildReadResponseWithEtag(
+        manager::getCurrentDynamicConfigBytes,
+        manager::convertBytesToDynamicConfig
+    );
   }
 
   // default value is used for backwards compatibility
diff --git a/server/src/main/java/org/apache/druid/server/http/DynamicConfigEtagHelper.java b/server/src/main/java/org/apache/druid/server/http/DynamicConfigEtagHelper.java
@@ -19,14 +19,19 @@
 
 package org.apache.druid.server.http;
 
+import com.google.common.base.Throwables;
 import org.apache.druid.common.config.ConfigEtag;
 import org.apache.druid.common.config.ConfigManager.SetResult;
+import org.apache.druid.error.DruidException;
+import org.apache.druid.error.InternalServerError;
 import org.apache.druid.error.InvalidInput;
 
 import javax.annotation.Nullable;
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
 /**
  * HTTP-layer helpers for {@code ETag} / {@code If-Match} on dynamic-config endpoints.
@@ -67,6 +72,29 @@ public static Response.ResponseBuilder withEtag(Response.ResponseBuilder builder
     return builder;
   }
 
+  /**
+   * Build a read response whose entity and ETag are both derived from one read
+   * of the stored config bytes.
+   */
+  public static <T> Response buildReadResponseWithEtag(
+      Supplier<byte[]> bytesSupplier,
+      Function<byte[], T> entitySupplier
+  )
+  {
+    try {
+      final byte[] currentBytes = bytesSupplier.get();
+      return withEtag(Response.ok(entitySupplier.apply(currentBytes)), currentBytes).build();
+    }
+    catch (DruidException e) {
+      return ServletResourceUtils.buildErrorResponseFrom(e);
+    }
+    catch (Exception e) {
+      return ServletResourceUtils.buildErrorResponseFrom(
+          InternalServerError.exception(Throwables.getRootCause(e), "Unknown error occurred")
+      );
+    }
+  }
+
   /**
    * Map a failed {@link SetResult} to 412 (precondition failed) or 400.
    * Callers handle the success case themselves.
diff --git a/server/src/test/java/org/apache/druid/server/http/CoordinatorBrokerConfigsResourceTest.java b/server/src/test/java/org/apache/druid/server/http/CoordinatorBrokerConfigsResourceTest.java
@@ -22,9 +22,11 @@
 import com.google.common.collect.ImmutableList;
 import org.apache.druid.audit.AuditInfo;
 import org.apache.druid.audit.AuditManager;
+import org.apache.druid.common.config.ConfigEtag;
 import org.apache.druid.common.config.ConfigManager.SetResult;
 import org.apache.druid.common.config.JacksonConfigManager;
 import org.apache.druid.error.ErrorResponse;
+import org.apache.druid.query.QueryContext;
 import org.apache.druid.server.broker.BrokerDynamicConfig;
 import org.easymock.EasyMock;
 import org.junit.Assert;
@@ -34,6 +36,8 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
 
 public class CoordinatorBrokerConfigsResourceTest
@@ -53,8 +57,12 @@ public void setUp() throws Exception
   @Test
   public void testGetBrokerDynamicConfig()
   {
-    BrokerDynamicConfig config = BrokerDynamicConfig.builder().build();
-    AtomicReference<BrokerDynamicConfig> currentConfig = new AtomicReference<>(config);
+    final BrokerDynamicConfig watchedConfig = BrokerDynamicConfig.builder().build();
+    final BrokerDynamicConfig config = BrokerDynamicConfig.builder()
+                                                          .withQueryContext(QueryContext.of(Map.of("priority", 5)))
+                                                          .build();
+    final AtomicReference<BrokerDynamicConfig> currentConfig = new AtomicReference<>(watchedConfig);
+    final byte[] currentBytes = "current-broker-config".getBytes(StandardCharsets.UTF_8);
 
     EasyMock.expect(
         configManager.watch(
@@ -63,8 +71,16 @@ public void testGetBrokerDynamicConfig()
             EasyMock.anyObject(BrokerDynamicConfig.class)
         )
     ).andReturn(currentConfig).once();
-    EasyMock.expect(configManager.getCurrentBytes(EasyMock.anyObject(String.class)))
-            .andReturn(null).once();
+    EasyMock.expect(configManager.getCurrentBytes(BrokerDynamicConfig.CONFIG_KEY))
+            .andReturn(currentBytes)
+            .once();
+    EasyMock.expect(
+        configManager.convertByteToConfig(
+            EasyMock.aryEq(currentBytes),
+            EasyMock.eq(BrokerDynamicConfig.class),
+            EasyMock.anyObject(BrokerDynamicConfig.class)
+        )
+    ).andReturn(config).once();
 
     EasyMock.replay(configManager, auditManager, brokerDynamicConfigSyncer);
 
@@ -76,6 +92,7 @@ public void testGetBrokerDynamicConfig()
 
     Assert.assertEquals(200, response.getStatus());
     Assert.assertEquals(config, response.getEntity());
+    Assert.assertEquals(ConfigEtag.compute(currentBytes), response.getMetadata().getFirst(HttpHeaders.ETAG));
 
     EasyMock.verify(configManager, auditManager, brokerDynamicConfigSyncer);
   }
diff --git a/server/src/test/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResourceTest.java b/server/src/test/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResourceTest.java
diff --git a/server/src/test/java/org/apache/druid/server/http/CoordinatorDynamicConfigsResourceTest.java b/server/src/test/java/org/apache/druid/server/http/CoordinatorDynamicConfigsResourceTest.java

Original file line number	Diff line number	Diff line change
`@@ -88,6 +88,15 @@ public CoordinatorDynamicConfig getCurrentDynamicConfig()`
`88`	`88`	`return Preconditions.checkNotNull(dynamicConfig, "Got null config from watcher?!");`
`89`	`89`	`}`
`90`	`90`
	`91`	`+ public CoordinatorDynamicConfig convertBytesToDynamicConfig(@Nullable byte[] bytes)`
	`92`	`+ {`
	`93`	`+ return jacksonConfigManager.convertByteToConfig(`
	`94`	`+ bytes,`
	`95`	`+ CoordinatorDynamicConfig.class,`
	`96`	`+ CoordinatorDynamicConfig.builder().build()`
	`97`	`+ );`
	`98`	`+ }`
	`99`	`+`
`91`	`100`	`public ConfigManager.SetResult setDynamicConfig(CoordinatorDynamicConfig config, AuditInfo auditInfo)`
`92`	`101`	`{`
`93`	`102`	`return setDynamicConfig(config, null, auditInfo);`
`@@ -190,7 +199,7 @@ public ConfigManager.SetResult getAndUpdateCompactionConfig(`
`190`	`199`	`}`
`191`	`200`	`}`
`192`	`201`
`193`		`- public DruidCompactionConfig convertBytesToCompactionConfig(byte[] bytes)`
	`202`	`+ public DruidCompactionConfig convertBytesToCompactionConfig(@Nullable byte[] bytes)`
`194`	`203`	`{`
`195`	`204`	`return jacksonConfigManager.convertByteToConfig(`
`196`	`205`	`bytes,`
Original file line number	Diff line number	Diff line change
`@@ -75,10 +75,10 @@ public CoordinatorDynamicConfigsResource(`
`75`	`75`	`@Produces(MediaType.APPLICATION_JSON)`
`76`	`76`	`public Response getDynamicConfigs()`
`77`	`77`	`{`
`78`		`- return DynamicConfigEtagHelper.withEtag(`
`79`		`- Response.ok(manager.getCurrentDynamicConfig()),`
`80`		`- manager.getCurrentDynamicConfigBytes()`
`81`		`- ).build();`
	`78`	`+ return DynamicConfigEtagHelper.buildReadResponseWithEtag(`
	`79`	`+ manager::getCurrentDynamicConfigBytes,`
	`80`	`+ manager::convertBytesToDynamicConfig`
	`81`	`+ );`
`82`	`82`	`}`
`83`	`83`
`84`	`84`	`// default value is used for backwards compatibility`