From c7ee5692ea50fc1c5bd72d82a0d976b9d43132e3 Mon Sep 17 00:00:00 2001 From: "Lucas.Capistrant" Date: Tue, 7 Jun 2022 10:30:59 -0500 Subject: [PATCH 1/4] Modify process for deserializing DynamicCoordinatorConfig --- .../server/coordinator/CoordinatorDynamicConfig.java | 9 +++++---- .../server/coordinator/DruidCoordinatorTest.java | 11 +++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) 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 90574780eabe..766b6bbad888 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 @@ -216,19 +216,20 @@ private static Set parseJsonStringOrArray(Object jsonStringOrArray) } } - public static AtomicReference watch(final JacksonConfigManager configManager) + public static AtomicReference watch(final JacksonConfigManager configManager) { return configManager.watch( CoordinatorDynamicConfig.CONFIG_KEY, - CoordinatorDynamicConfig.class, - CoordinatorDynamicConfig.builder().build() + CoordinatorDynamicConfig.Builder.class, + CoordinatorDynamicConfig.builder() ); } @Nonnull public static CoordinatorDynamicConfig current(final JacksonConfigManager configManager) { - return Preconditions.checkNotNull(watch(configManager).get(), "Got null config from watcher?!"); + Preconditions.checkNotNull(watch(configManager).get(), "Got null config from watcher?!"); + return watch(configManager).get().build(); } @JsonProperty("millisToWaitBeforeDeleting") diff --git a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java index 26975d9aa9dd..0c83164a735e 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/DruidCoordinatorTest.java @@ -131,7 +131,7 @@ public void setUp() throws Exception EasyMock.anyObject(Class.class), EasyMock.anyObject() ) - ).andReturn(new AtomicReference(CoordinatorDynamicConfig.builder().build())).anyTimes(); + ).andReturn(new AtomicReference(CoordinatorDynamicConfig.builder())).anyTimes(); EasyMock.expect( configManager.watch( EasyMock.eq(CoordinatorCompactionConfig.CONFIG_KEY), @@ -729,6 +729,9 @@ public void testBalancerThreadNumber() EasyMock.expect(dynamicConfig.getBalancerComputeThreads()).andReturn(5).times(2); EasyMock.expect(dynamicConfig.getBalancerComputeThreads()).andReturn(10).once(); + CoordinatorDynamicConfig.Builder dynamicConfigBuilder = EasyMock.createNiceMock(CoordinatorDynamicConfig.Builder.class); + EasyMock.expect(dynamicConfigBuilder.build()).andReturn(dynamicConfig).anyTimes(); + JacksonConfigManager configManager = EasyMock.createNiceMock(JacksonConfigManager.class); EasyMock.expect( configManager.watch( @@ -736,10 +739,10 @@ public void testBalancerThreadNumber() EasyMock.anyObject(Class.class), EasyMock.anyObject() ) - ).andReturn(new AtomicReference(dynamicConfig)).anyTimes(); + ).andReturn(new AtomicReference(dynamicConfigBuilder)).anyTimes(); ScheduledExecutorFactory scheduledExecutorFactory = EasyMock.createNiceMock(ScheduledExecutorFactory.class); - EasyMock.replay(configManager, dynamicConfig, scheduledExecutorFactory); + EasyMock.replay(configManager, dynamicConfig, scheduledExecutorFactory, dynamicConfigBuilder); DruidCoordinator c = new DruidCoordinator( druidCoordinatorConfig, @@ -1025,7 +1028,7 @@ public void testCoordinatorCustomDutyGroupsRunAsExpected() throws Exception EasyMock.anyObject(Class.class), EasyMock.anyObject() ) - ).andReturn(new AtomicReference(CoordinatorDynamicConfig.builder().build())).anyTimes(); + ).andReturn(new AtomicReference(CoordinatorDynamicConfig.builder())).anyTimes(); EasyMock.expect( configManager.watch( EasyMock.eq(CoordinatorCompactionConfig.CONFIG_KEY), From 0b5e06c326808f6a1d2a487da803144043b18da5 Mon Sep 17 00:00:00 2001 From: "Lucas.Capistrant" Date: Tue, 7 Jun 2022 14:08:11 -0500 Subject: [PATCH 2/4] small code cleanup and a javadoc on changed method --- .../coordinator/CoordinatorDynamicConfig.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) 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 766b6bbad888..558d8a381723 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 @@ -216,6 +216,16 @@ private static Set parseJsonStringOrArray(Object jsonStringOrArray) } } + /** + * Setup a watch on the CoordinatorDynamicConfig. + * + * Note that the CoordinatorDynamicConfig.Builder class is used here for serde because that allows clean setting of + * defaults for missing configuation keys. Missing configuration keys are common in cases such as the addition of a + * new config key in a new Druid version. + * + * @param configManager responsible for managing the reading/writing of druid configs to the metastore. + * @return AtomicReference to CoordinatorDyamicConfig.Builder + */ public static AtomicReference watch(final JacksonConfigManager configManager) { return configManager.watch( @@ -228,8 +238,7 @@ public static AtomicReference watch(final Jack @Nonnull public static CoordinatorDynamicConfig current(final JacksonConfigManager configManager) { - Preconditions.checkNotNull(watch(configManager).get(), "Got null config from watcher?!"); - return watch(configManager).get().build(); + return Preconditions.checkNotNull(watch(configManager).get().build(), "Got null config from watcher?!"); } @JsonProperty("millisToWaitBeforeDeleting") From 6d0bc49a32702cadd00c67cf17f63eebf23bdd7c Mon Sep 17 00:00:00 2001 From: "Lucas.Capistrant" Date: Tue, 7 Jun 2022 14:13:36 -0500 Subject: [PATCH 3/4] update java doc --- .../server/coordinator/CoordinatorDynamicConfig.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 558d8a381723..70f103045e1d 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 @@ -217,14 +217,14 @@ private static Set parseJsonStringOrArray(Object jsonStringOrArray) } /** - * Setup a watch on the CoordinatorDynamicConfig. + * Setup a watch on the {@link CoordinatorDynamicConfig}. * - * Note that the CoordinatorDynamicConfig.Builder class is used here for serde because that allows clean setting of + * Note that the {@link CoordinatorDynamicConfig.Builder} class is used here for serde because that allows clean setting of * defaults for missing configuation keys. Missing configuration keys are common in cases such as the addition of a * new config key in a new Druid version. * - * @param configManager responsible for managing the reading/writing of druid configs to the metastore. - * @return AtomicReference to CoordinatorDyamicConfig.Builder + * @param configManager {@link JacksonConfigManager } responsible for managing persisted druid configuration content + * @return {@link AtomicReference} of {@link CoordinatorDynamicConfig.Builder} */ public static AtomicReference watch(final JacksonConfigManager configManager) { From 60b2410606a79cb425e42da38aab6ffe0be80b2e Mon Sep 17 00:00:00 2001 From: "Lucas.Capistrant" Date: Tue, 7 Jun 2022 14:14:48 -0500 Subject: [PATCH 4/4] edit javadoc --- .../druid/server/coordinator/CoordinatorDynamicConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 70f103045e1d..6e073e678e35 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 @@ -217,7 +217,7 @@ private static Set parseJsonStringOrArray(Object jsonStringOrArray) } /** - * Setup a watch on the {@link CoordinatorDynamicConfig}. + * Setup a watch on the {@link CoordinatorDynamicConfig} in order to ensure access to the latest stored version of the config. * * Note that the {@link CoordinatorDynamicConfig.Builder} class is used here for serde because that allows clean setting of * defaults for missing configuation keys. Missing configuration keys are common in cases such as the addition of a