You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2018/11/22 02:25:41 UTC
[GitHub] asdf2014 closed pull request #6652: Fix missing default config in
some calls to coordinator dynamic configs.
asdf2014 closed pull request #6652: Fix missing default config in some calls to coordinator dynamic configs.
URL: https://github.com/apache/incubator-druid/pull/6652
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorCompactionConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorCompactionConfig.java
index bb16e438215..cc1fdf61e34 100644
--- a/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorCompactionConfig.java
+++ b/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorCompactionConfig.java
@@ -21,11 +21,15 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import org.apache.druid.common.config.JacksonConfigManager;
+import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.List;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
public class CoordinatorCompactionConfig
{
@@ -73,6 +77,21 @@ public static CoordinatorCompactionConfig empty()
return new CoordinatorCompactionConfig(ImmutableList.of(), null, null);
}
+ public static AtomicReference<CoordinatorCompactionConfig> watch(final JacksonConfigManager configManager)
+ {
+ return configManager.watch(
+ CoordinatorCompactionConfig.CONFIG_KEY,
+ CoordinatorCompactionConfig.class,
+ CoordinatorCompactionConfig.empty()
+ );
+ }
+
+ @Nonnull
+ public static CoordinatorCompactionConfig current(final JacksonConfigManager configManager)
+ {
+ return Preconditions.checkNotNull(watch(configManager).get(), "Got null config from watcher?!");
+ }
+
@JsonCreator
public CoordinatorCompactionConfig(
@JsonProperty("compactionConfigs") List<DataSourceCompactionConfig> compactionConfigs,
diff --git a/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorDynamicConfig.java b/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorDynamicConfig.java
index 8e8df480bdf..7ec2b095544 100644
--- a/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorDynamicConfig.java
+++ b/server/src/main/java/org/apache/druid/server/coordinator/CoordinatorDynamicConfig.java
@@ -20,21 +20,25 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
+import org.apache.druid.common.config.JacksonConfigManager;
import org.apache.druid.java.util.common.IAE;
+import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.Collection;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
/**
* This class is for users to change their configurations while their Druid cluster is running.
* These configurations are designed to allow only simple values rather than complicated JSON objects.
*
- * @see org.apache.druid.common.config.JacksonConfigManager
+ * @see JacksonConfigManager
* @see org.apache.druid.common.config.ConfigManager
*/
public class CoordinatorDynamicConfig
@@ -120,6 +124,21 @@ public CoordinatorDynamicConfig(
}
}
+ public static AtomicReference<CoordinatorDynamicConfig> watch(final JacksonConfigManager configManager)
+ {
+ return configManager.watch(
+ CoordinatorDynamicConfig.CONFIG_KEY,
+ CoordinatorDynamicConfig.class,
+ CoordinatorDynamicConfig.builder().build()
+ );
+ }
+
+ @Nonnull
+ public static CoordinatorDynamicConfig current(final JacksonConfigManager configManager)
+ {
+ return Preconditions.checkNotNull(watch(configManager).get(), "Got null config from watcher?!");
+ }
+
@JsonProperty
public long getMillisToWaitBeforeDeleting()
{
@@ -423,7 +442,9 @@ public CoordinatorDynamicConfig build()
killDataSourceWhitelist,
killAllDataSources == null ? DEFAULT_KILL_ALL_DATA_SOURCES : killAllDataSources,
killPendingSegmentsSkipList,
- maxSegmentsInNodeLoadingQueue == null ? DEFAULT_MAX_SEGMENTS_IN_NODE_LOADING_QUEUE : maxSegmentsInNodeLoadingQueue
+ maxSegmentsInNodeLoadingQueue == null
+ ? DEFAULT_MAX_SEGMENTS_IN_NODE_LOADING_QUEUE
+ : maxSegmentsInNodeLoadingQueue
);
}
@@ -441,7 +462,9 @@ public CoordinatorDynamicConfig build(CoordinatorDynamicConfig defaults)
killDataSourceWhitelist == null ? defaults.getKillDataSourceWhitelist() : killDataSourceWhitelist,
killAllDataSources == null ? defaults.isKillAllDataSources() : killAllDataSources,
killPendingSegmentsSkipList == null ? defaults.getKillPendingSegmentsSkipList() : killPendingSegmentsSkipList,
- maxSegmentsInNodeLoadingQueue == null ? defaults.getMaxSegmentsInNodeLoadingQueue() : maxSegmentsInNodeLoadingQueue
+ maxSegmentsInNodeLoadingQueue == null
+ ? defaults.getMaxSegmentsInNodeLoadingQueue()
+ : maxSegmentsInNodeLoadingQueue
);
}
}
diff --git a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java
index 8bf9ed5b448..852068342c0 100644
--- a/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java
+++ b/server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java
@@ -324,20 +324,12 @@ public long remainingSegmentSizeBytesForCompaction(String dataSource)
public CoordinatorDynamicConfig getDynamicConfigs()
{
- return configManager.watch(
- CoordinatorDynamicConfig.CONFIG_KEY,
- CoordinatorDynamicConfig.class,
- CoordinatorDynamicConfig.builder().build()
- ).get();
+ return CoordinatorDynamicConfig.current(configManager);
}
public CoordinatorCompactionConfig getCompactionConfig()
{
- return configManager.watch(
- CoordinatorCompactionConfig.CONFIG_KEY,
- CoordinatorCompactionConfig.class,
- CoordinatorCompactionConfig.empty()
- ).get();
+ return CoordinatorCompactionConfig.current(configManager);
}
public void removeSegment(DataSegment segment)
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
index 46044037bc8..8f7bf26721b 100644
--- a/server/src/main/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResource.java
+++ b/server/src/main/java/org/apache/druid/server/http/CoordinatorCompactionConfigsResource.java
@@ -65,12 +65,7 @@ public CoordinatorCompactionConfigsResource(JacksonConfigManager manager)
@Produces(MediaType.APPLICATION_JSON)
public Response getCompactConfig()
{
- return Response.ok(
- manager.watch(
- CoordinatorCompactionConfig.CONFIG_KEY,
- CoordinatorCompactionConfig.class
- ).get()
- ).build();
+ return Response.ok(CoordinatorCompactionConfig.current(manager)).build();
}
@POST
@@ -84,17 +79,13 @@ public Response setCompactionTaskLimit(
@Context HttpServletRequest req
)
{
- CoordinatorCompactionConfig current = manager.watch(
- CoordinatorCompactionConfig.CONFIG_KEY,
- CoordinatorCompactionConfig.class
- ).get();
+ final CoordinatorCompactionConfig current = CoordinatorCompactionConfig.current(manager);
- final CoordinatorCompactionConfig newCompactionConfig;
- if (current != null) {
- newCompactionConfig = CoordinatorCompactionConfig.from(current, compactionTaskSlotRatio, maxCompactionTaskSlots);
- } else {
- newCompactionConfig = new CoordinatorCompactionConfig(null, compactionTaskSlotRatio, maxCompactionTaskSlots);
- }
+ final CoordinatorCompactionConfig newCompactionConfig = CoordinatorCompactionConfig.from(
+ current,
+ compactionTaskSlotRatio,
+ maxCompactionTaskSlots
+ );
final SetResult setResult = manager.set(
CoordinatorCompactionConfig.CONFIG_KEY,
@@ -120,22 +111,14 @@ public Response addOrUpdateCompactionConfig(
@Context HttpServletRequest req
)
{
- CoordinatorCompactionConfig current = manager.watch(
- CoordinatorCompactionConfig.CONFIG_KEY,
- CoordinatorCompactionConfig.class
- ).get();
-
+ final CoordinatorCompactionConfig current = CoordinatorCompactionConfig.current(manager);
final CoordinatorCompactionConfig newCompactionConfig;
- if (current != null) {
- final Map<String, DataSourceCompactionConfig> newConfigs = current
- .getCompactionConfigs()
- .stream()
- .collect(Collectors.toMap(DataSourceCompactionConfig::getDataSource, Function.identity()));
- newConfigs.put(newConfig.getDataSource(), newConfig);
- newCompactionConfig = CoordinatorCompactionConfig.from(current, ImmutableList.copyOf(newConfigs.values()));
- } else {
- newCompactionConfig = CoordinatorCompactionConfig.from(ImmutableList.of(newConfig));
- }
+ final Map<String, DataSourceCompactionConfig> newConfigs = current
+ .getCompactionConfigs()
+ .stream()
+ .collect(Collectors.toMap(DataSourceCompactionConfig::getDataSource, Function.identity()));
+ newConfigs.put(newConfig.getDataSource(), newConfig);
+ newCompactionConfig = CoordinatorCompactionConfig.from(current, ImmutableList.copyOf(newConfigs.values()));
final SetResult setResult = manager.set(
CoordinatorCompactionConfig.CONFIG_KEY,
@@ -155,15 +138,7 @@ public Response addOrUpdateCompactionConfig(
@Produces(MediaType.APPLICATION_JSON)
public Response getCompactionConfig(@PathParam("dataSource") String dataSource)
{
- CoordinatorCompactionConfig current = manager.watch(
- CoordinatorCompactionConfig.CONFIG_KEY,
- CoordinatorCompactionConfig.class
- ).get();
-
- if (current == null) {
- return Response.status(Response.Status.NOT_FOUND).build();
- }
-
+ final CoordinatorCompactionConfig current = CoordinatorCompactionConfig.current(manager);
final Map<String, DataSourceCompactionConfig> configs = current
.getCompactionConfigs()
.stream()
@@ -187,15 +162,7 @@ public Response deleteCompactionConfig(
@Context HttpServletRequest req
)
{
- CoordinatorCompactionConfig current = manager.watch(
- CoordinatorCompactionConfig.CONFIG_KEY,
- CoordinatorCompactionConfig.class
- ).get();
-
- if (current == null) {
- return Response.status(Response.Status.NOT_FOUND).build();
- }
-
+ final CoordinatorCompactionConfig current = CoordinatorCompactionConfig.current(manager);
final Map<String, DataSourceCompactionConfig> configs = current
.getCompactionConfigs()
.stream()
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
index 1372d85e38e..85bdd3320bc 100644
--- a/server/src/main/java/org/apache/druid/server/http/CoordinatorDynamicConfigsResource.java
+++ b/server/src/main/java/org/apache/druid/server/http/CoordinatorDynamicConfigsResource.java
@@ -67,31 +67,24 @@ public CoordinatorDynamicConfigsResource(
@Produces(MediaType.APPLICATION_JSON)
public Response getDynamicConfigs()
{
- return Response.ok(
- manager.watch(
- CoordinatorDynamicConfig.CONFIG_KEY,
- CoordinatorDynamicConfig.class
- ).get()
- ).build();
+ return Response.ok(CoordinatorDynamicConfig.current(manager)).build();
}
// default value is used for backwards compatibility
@POST
@Consumes(MediaType.APPLICATION_JSON)
- public Response setDynamicConfigs(final CoordinatorDynamicConfig.Builder dynamicConfigBuilder,
- @HeaderParam(AuditManager.X_DRUID_AUTHOR) @DefaultValue("") final String author,
- @HeaderParam(AuditManager.X_DRUID_COMMENT) @DefaultValue("") final String comment,
- @Context HttpServletRequest req
+ public Response setDynamicConfigs(
+ final CoordinatorDynamicConfig.Builder dynamicConfigBuilder,
+ @HeaderParam(AuditManager.X_DRUID_AUTHOR) @DefaultValue("") final String author,
+ @HeaderParam(AuditManager.X_DRUID_COMMENT) @DefaultValue("") final String comment,
+ @Context HttpServletRequest req
)
{
- CoordinatorDynamicConfig current = manager.watch(
- CoordinatorDynamicConfig.CONFIG_KEY,
- CoordinatorDynamicConfig.class
- ).get();
+ CoordinatorDynamicConfig current = CoordinatorDynamicConfig.current(manager);
final SetResult setResult = manager.set(
CoordinatorDynamicConfig.CONFIG_KEY,
- current == null ? dynamicConfigBuilder.build() : dynamicConfigBuilder.build(current),
+ dynamicConfigBuilder.build(current),
new AuditInfo(author, comment, req.getRemoteAddr())
);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org