jihoonson commented on a change in pull request #10363:
URL: https://github.com/apache/druid/pull/10363#discussion_r500681350
##########
File path: server/src/main/java/org/apache/druid/guice/StorageNodeModule.java
##########
@@ -52,6 +55,7 @@ public void configure(Binder binder)
{
JsonConfigProvider.bind(binder, "druid.server", DruidServerConfig.class);
JsonConfigProvider.bind(binder, "druid.segmentCache",
SegmentLoaderConfig.class);
+ JsonConfigProvider.bind(binder,
"druid.segmentCache.locationSelector.strategy",
StorageLocationSelectorStrategy.class);
Review comment:
Should this be `druid.segmentCache.locationSelector` instead?
##########
File path:
server/src/test/java/org/apache/druid/segment/loading/StorageLocationSelectorStrategyTest.java
##########
@@ -256,4 +270,107 @@ public void
testMostAvailableSizeLocationSelectorStrategy() throws Exception
Assert.assertEquals("The next element of the iterator should point to path
local_storage_folder_1",
localStorageFolder1, loc3.getPath());
}
+
+ @Test
+ public void testDefaultSelectorStrategyConfig()
+ {
+ //no druid.segmentCache.locationSelector.strategy specified, the default
will be used
+ final Properties props = new Properties();
+ props.setProperty("druid.segmentCache.locations", "[{\"path\":
\"/tmp/druid/indexCache\"}]");
+
+ StorageLocationSelectorStrategy strategy =
makeInjectorWithProperties(props).getInstance(StorageLocationSelectorStrategy.class);
+ Assert.assertEquals(LeastBytesUsedStorageLocationSelectorStrategy.class,
+ strategy.getClass());
+ Assert.assertEquals("/tmp/druid/indexCache",
strategy.getLocations().next().getPath().getAbsolutePath());
+ }
+
+ @Test
+ public void testRoundRobinSelectorStrategyConfig()
+ {
+ final Properties props = new Properties();
+ props.setProperty("druid.segmentCache.locations", "[{\"path\":
\"/tmp/druid/indexCache\"}]");
+ props.setProperty("druid.segmentCache.locationSelector.strategy",
"roundRobin");
+
+ Injector injector = makeInjectorWithProperties(props);
+ StorageLocationSelectorStrategy strategy =
injector.getInstance(StorageLocationSelectorStrategy.class);
+
+ Assert.assertEquals(RoundRobinStorageLocationSelectorStrategy.class,
+ strategy.getClass());
+ Assert.assertEquals("/tmp/druid/indexCache",
strategy.getLocations().next().getPath().getAbsolutePath());
+ }
+
+ @Test
+ public void testLeastBytesUsedSelectorStrategyConfig()
+ {
+ final Properties props = new Properties();
+ props.setProperty("druid.segmentCache.locations", "[{\"path\":
\"/tmp/druid/indexCache\"}]");
+ props.setProperty("druid.segmentCache.locationSelector.strategy",
"leastBytesUsed");
+
+ Injector injector = makeInjectorWithProperties(props);
+ StorageLocationSelectorStrategy strategy =
injector.getInstance(StorageLocationSelectorStrategy.class);
+
+ Assert.assertEquals(LeastBytesUsedStorageLocationSelectorStrategy.class,
+ strategy.getClass());
+ Assert.assertEquals("/tmp/druid/indexCache",
strategy.getLocations().next().getPath().getAbsolutePath());
+ }
+
+ @Test
+ public void testRandomSelectorStrategyConfig()
+ {
+ final Properties props = new Properties();
+ props.setProperty("druid.segmentCache.locations", "[{\"path\":
\"/tmp/druid/indexCache\"}]");
+ props.setProperty("druid.segmentCache.locationSelector.strategy",
"random");
+
+ Injector injector = makeInjectorWithProperties(props);
+ StorageLocationSelectorStrategy strategy =
injector.getInstance(StorageLocationSelectorStrategy.class);
+
+ Assert.assertEquals(RandomStorageLocationSelectorStrategy.class,
+ strategy.getClass());
+ Assert.assertEquals("/tmp/druid/indexCache",
strategy.getLocations().next().getPath().getAbsolutePath());
+ }
+
+ @Test
+ public void testMostAvailableSizeSelectorStrategyConfig()
+ {
+ final Properties props = new Properties();
+ props.setProperty("druid.segmentCache.locationSelector.strategy",
"mostAvailableSize");
+ props.setProperty("druid.segmentCache.locations", "[{\"path\":
\"/tmp/druid/indexCache\"}]");
+
+ Injector injector = makeInjectorWithProperties(props);
+ StorageLocationSelectorStrategy strategy =
injector.getInstance(StorageLocationSelectorStrategy.class);
+
+ Assert.assertEquals(MostAvailableSizeStorageLocationSelectorStrategy.class,
+ strategy.getClass());
+ Assert.assertEquals("/tmp/druid/indexCache",
strategy.getLocations().next().getPath().getAbsolutePath());
+ }
+
+ private Injector makeInjectorWithProperties(final Properties props)
+ {
+ return Guice.createInjector(
+ new Module()
+ {
+ @Override
+ public void configure(Binder binder)
+ {
+ //ObjectMapperModule introduce Guice injector for jackson
+ binder.install(new ObjectMapperModule()
+ .withObjectMapper(new DefaultObjectMapper()));
+ binder.install(new DruidGuiceExtensions());
+
+
binder.bind(Validator.class).toInstance(Validation.buildDefaultValidatorFactory().getValidator());
+ binder.bind(JsonConfigurator.class).in(LazySingleton.class);
+ binder.bind(Properties.class).toInstance(props);
+
+ JsonConfigProvider.bind(binder, "druid.segmentCache",
SegmentLoaderConfig.class);
+ JsonConfigProvider.bind(binder,
"druid.segmentCache.locationSelector", StorageLocationSelectorStrategy.class);
Review comment:
This binding is different from what actual module binds. I think this is
why we missed the wrong binding. Can we use the same `StorageNodeModule` here?
Or can we add a helper method which does the proper binding for both
`StorageNodeModule` and tests?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
