[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r19983
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/ActionOnFile.java
##
@@ -0,0 +1,68 @@
+package org.apache.drill.exec.store;
+
+import org.apache.drill.common.config.CommonConstants;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+
+/**
+ * The action on the {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
file being performed after it's use
+ */
+public enum ActionOnFile {
+
+ NONE {
+@Override
+void action(URL url) {
+ // nothing to do
+}
+ },
+ RENAME {
+@Override
+void action(URL url) {
+ File pluginsOverrideFile = new File(url.getPath());
+ String oldName = CommonConstants.STORAGE_PLUGINS_OVERRIDE_CONF;
+ String currentDateTime = new
SimpleDateFormat("MMddHHmmss").format(new Date());
+ String newFileName = new StringBuilder(oldName)
+ .insert(oldName.lastIndexOf("."), "-" + currentDateTime)
+ .toString();
+ Path pluginsOverrideFilePath = pluginsOverrideFile.toPath();
+ try {
+Files.move(pluginsOverrideFilePath,
pluginsOverrideFilePath.resolveSibling(newFileName));
+ } catch (IOException e) {
+logger.error("%s file is not renamed after it's use",
CommonConstants.STORAGE_PLUGINS_OVERRIDE_CONF, e);
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199844395
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/ActionOnFile.java
##
@@ -0,0 +1,68 @@
+package org.apache.drill.exec.store;
+
+import org.apache.drill.common.config.CommonConstants;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+
+/**
+ * The action on the {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
file being performed after it's use
+ */
+public enum ActionOnFile {
+
+ NONE {
+@Override
+void action(URL url) {
+ // nothing to do
+}
+ },
+ RENAME {
+@Override
+void action(URL url) {
+ File pluginsOverrideFile = new File(url.getPath());
+ String oldName = CommonConstants.STORAGE_PLUGINS_OVERRIDE_CONF;
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199844868
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandlerService.java
##
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+import java.io.IOException;
+import java.net.URL;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import static
org.apache.drill.exec.store.StoragePluginRegistry.ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF} conf file
+ *
+ * TODO: DRILL-6564: It can be improved with configs versioning and service of
creating
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
+ */
+public class StoragePluginsHandlerService implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsHandlerService.class);
+
+ private final LogicalPlanPersistence lpPersistence;
+ private final DrillbitContext context;
+ private URL pluginsOverrideFileUrl;
+
+ public StoragePluginsHandlerService(DrillbitContext context) {
+this.context = context;
+this.lpPersistence = new LogicalPlanPersistence(context.getConfig(),
context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public void loadPlugins(@NotNull PersistentStore
persistentStore,
+ @Nullable StoragePlugins bootstrapPlugins) {
+// if bootstrapPlugins is not null -- fresh Drill set up
+StoragePlugins pluginsForPersistentStore;
+
+StoragePlugins newPlugins = getNewStoragePlugins();
+
+if (newPlugins != null) {
+ pluginsForPersistentStore = new StoragePlugins(new HashMap<>());
+ Optional.ofNullable(bootstrapPlugins)
+ .ifPresent(pluginsForPersistentStore::putAll);
+
+ for (Map.Entry newPlugin : newPlugins) {
+String pluginName = newPlugin.getKey();
+StoragePluginConfig oldPluginConfig =
Optional.ofNullable(bootstrapPlugins)
+.map(plugins -> plugins.getConfig(pluginName))
+.orElse(persistentStore.get(pluginName));
+StoragePluginConfig updatedStatusPluginConfig =
updatePluginStatus(oldPluginConfig, newPlugin.getValue());
+pluginsForPersistentStore.put(pluginName, updatedStatusPluginConfig);
+ }
+} else {
+ pluginsForPersistentStore = bootstrapPlugins;
+}
+
+// load pluginsForPersistentStore to Persistent Store
+Optional.ofNullable(pluginsForPersistentStore)
+.ifPresent(plugins -> plugins.forEach(plugin ->
persistentStore.put(plugin.getKey(), plugin.getValue(;
+
+if (newPlugins != null) {
+ String fileAction =
context.getConfig().getString(ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE).toUpperCase();
+ Optional actionOnFile =
Arrays.stream(ActionOnFile.values())
+ .filter(action -> action.name().equals(fileAction))
Review comment:
Done
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.
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199844308
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/ActionOnFile.java
##
@@ -0,0 +1,68 @@
+package org.apache.drill.exec.store;
+
+import org.apache.drill.common.config.CommonConstants;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+
+/**
+ * The action on the {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
file being performed after it's use
+ */
+public enum ActionOnFile {
+
+ NONE {
+@Override
+void action(URL url) {
+ // nothing to do
+}
+ },
+ RENAME {
+@Override
+void action(URL url) {
+ File pluginsOverrideFile = new File(url.getPath());
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199828971
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandlerService.java
##
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+import java.io.IOException;
+import java.net.URL;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import static
org.apache.drill.exec.store.StoragePluginRegistry.ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF} conf file
+ *
+ * TODO: DRILL-6564: It can be improved with configs versioning and service of
creating
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
+ */
+public class StoragePluginsHandlerService implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsHandlerService.class);
+
+ private final LogicalPlanPersistence lpPersistence;
+ private final DrillbitContext context;
+ private URL pluginsOverrideFileUrl;
+
+ public StoragePluginsHandlerService(DrillbitContext context) {
+this.context = context;
+this.lpPersistence = new LogicalPlanPersistence(context.getConfig(),
context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public void loadPlugins(@NotNull PersistentStore
persistentStore,
+ @Nullable StoragePlugins bootstrapPlugins) {
+// if bootstrapPlugins is not null -- fresh Drill set up
+StoragePlugins pluginsForPersistentStore;
+
+StoragePlugins newPlugins = getNewStoragePlugins();
+
+if (newPlugins != null) {
+ pluginsForPersistentStore = new StoragePlugins(new HashMap<>());
+ Optional.ofNullable(bootstrapPlugins)
+ .ifPresent(pluginsForPersistentStore::putAll);
+
+ for (Map.Entry newPlugin : newPlugins) {
+String pluginName = newPlugin.getKey();
+StoragePluginConfig oldPluginConfig =
Optional.ofNullable(bootstrapPlugins)
+.map(plugins -> plugins.getConfig(pluginName))
+.orElse(persistentStore.get(pluginName));
+StoragePluginConfig updatedStatusPluginConfig =
updatePluginStatus(oldPluginConfig, newPlugin.getValue());
+pluginsForPersistentStore.put(pluginName, updatedStatusPluginConfig);
+ }
+} else {
+ pluginsForPersistentStore = bootstrapPlugins;
+}
+
+// load pluginsForPersistentStore to Persistent Store
+Optional.ofNullable(pluginsForPersistentStore)
+.ifPresent(plugins -> plugins.forEach(plugin ->
persistentStore.put(plugin.getKey(), plugin.getValue(;
+
+if (newPlugins != null) {
+ String fileAction =
context.getConfig().getString(ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE).toUpperCase();
+ Optional actionOnFile =
Arrays.stream(ActionOnFile.values())
+ .filter(action -> action.name().equals(fileAction))
+ .findFirst();
+ actionOnFile.ifPresent(action -> action.action(pluginsOverrideFileUrl));
+ // TODO: replace with ifPresentOrElse() once the project will be on Java9
+ if (!actionOnFile.isPresent()) {
+logger.error("Unknown
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199745722
##
File path: distribution/src/resources/drill-override-example.conf
##
@@ -58,17 +58,8 @@ drill.exec: {
batch.size: 4000
},
partition.column.label: "dir"
-}
- },
- metrics : {
-context: "drillbit",
-jmx: {
- enabled : true
},
-log: {
- enabled : false,
- interval : 60
-}
+action_on_plugins_override_file: "rename"
Review comment:
The comment was in `drill-module.conf`, but you are right
`drill-override-example.conf` is better place for it.
Done.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199790967
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandlerService.java
##
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import static
org.apache.drill.exec.store.StoragePluginRegistry.ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF} conf file
+ *
+ * TODO: DRILL-6564: It can be improved with configs versioning and service of
creating
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
+ */
+public class StoragePluginsHandlerService implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsHandlerService.class);
+
+ private final LogicalPlanPersistence hoconLogicalPlanPersistence;
+ private final DrillbitContext context;
+ private URL pluginsOverrideFileUrl;
+
+ public StoragePluginsHandlerService(DrillbitContext context) {
+this.context = context;
+this.hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public void loadPlugins(@NotNull PersistentStore
persistentStore,
+ @Nullable StoragePlugins bootstrapPlugins) {
+// if bootstrapPlugins is not null -- fresh Drill set up
+StoragePlugins pluginsToBeWrittenToPersistentStore;
+
+StoragePlugins newPlugins = getNewStoragePlugins();
+
+if (newPlugins != null) {
+ pluginsToBeWrittenToPersistentStore = new StoragePlugins(new
HashMap<>());
+ Optional.ofNullable(bootstrapPlugins)
+ .ifPresent(pluginsToBeWrittenToPersistentStore::putAll);
+
+ for (Map.Entry newPlugin : newPlugins) {
+String pluginName = newPlugin.getKey();
+StoragePluginConfig oldPluginConfig =
Optional.ofNullable(bootstrapPlugins)
+.map(plugins -> plugins.getConfig(pluginName))
+.orElse(persistentStore.get(pluginName));
+StoragePluginConfig updatedStatusPluginConfig =
updatePluginStatus(oldPluginConfig, newPlugin.getValue());
+pluginsToBeWrittenToPersistentStore.put(pluginName,
updatedStatusPluginConfig);
+ }
+} else {
+ pluginsToBeWrittenToPersistentStore = bootstrapPlugins;
+}
+
+// load pluginsToBeWrittenToPersistentStore to Persistent Store
+Optional.ofNullable(pluginsToBeWrittenToPersistentStore)
+.ifPresent(plugins -> plugins.forEach(plugin ->
persistentStore.put(plugin.getKey(), plugin.getValue(;
+
+if (newPlugins != null) {
+ PluginsOverrideFileAction finalActionOnPluginsOverrideFile =
+
PluginsOverrideFileAction.valueOf(context.getConfig().getString(ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE).toUpperCase());
Review comment:
It will be logged as error now and no action will be performed.
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199789010
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandlerService.java
##
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import static
org.apache.drill.exec.store.StoragePluginRegistry.ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF} conf file
+ *
+ * TODO: DRILL-6564: It can be improved with configs versioning and service of
creating
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
+ */
+public class StoragePluginsHandlerService implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsHandlerService.class);
+
+ private final LogicalPlanPersistence hoconLogicalPlanPersistence;
+ private final DrillbitContext context;
+ private URL pluginsOverrideFileUrl;
+
+ public StoragePluginsHandlerService(DrillbitContext context) {
+this.context = context;
+this.hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public void loadPlugins(@NotNull PersistentStore
persistentStore,
+ @Nullable StoragePlugins bootstrapPlugins) {
+// if bootstrapPlugins is not null -- fresh Drill set up
+StoragePlugins pluginsToBeWrittenToPersistentStore;
+
+StoragePlugins newPlugins = getNewStoragePlugins();
+
+if (newPlugins != null) {
+ pluginsToBeWrittenToPersistentStore = new StoragePlugins(new
HashMap<>());
+ Optional.ofNullable(bootstrapPlugins)
+ .ifPresent(pluginsToBeWrittenToPersistentStore::putAll);
+
+ for (Map.Entry newPlugin : newPlugins) {
+String pluginName = newPlugin.getKey();
+StoragePluginConfig oldPluginConfig =
Optional.ofNullable(bootstrapPlugins)
+.map(plugins -> plugins.getConfig(pluginName))
+.orElse(persistentStore.get(pluginName));
+StoragePluginConfig updatedStatusPluginConfig =
updatePluginStatus(oldPluginConfig, newPlugin.getValue());
+pluginsToBeWrittenToPersistentStore.put(pluginName,
updatedStatusPluginConfig);
+ }
+} else {
+ pluginsToBeWrittenToPersistentStore = bootstrapPlugins;
+}
+
+// load pluginsToBeWrittenToPersistentStore to Persistent Store
+Optional.ofNullable(pluginsToBeWrittenToPersistentStore)
+.ifPresent(plugins -> plugins.forEach(plugin ->
persistentStore.put(plugin.getKey(), plugin.getValue(;
+
+if (newPlugins != null) {
+ PluginsOverrideFileAction finalActionOnPluginsOverrideFile =
+
PluginsOverrideFileAction.valueOf(context.getConfig().getString(ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE).toUpperCase());
+ switch (finalActionOnPluginsOverrideFile) {
+case RENAME:
+
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199790185
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandlerService.java
##
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import static
org.apache.drill.exec.store.StoragePluginRegistry.ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF} conf file
+ *
+ * TODO: DRILL-6564: It can be improved with configs versioning and service of
creating
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
+ */
+public class StoragePluginsHandlerService implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsHandlerService.class);
+
+ private final LogicalPlanPersistence hoconLogicalPlanPersistence;
+ private final DrillbitContext context;
+ private URL pluginsOverrideFileUrl;
+
+ public StoragePluginsHandlerService(DrillbitContext context) {
+this.context = context;
+this.hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public void loadPlugins(@NotNull PersistentStore
persistentStore,
+ @Nullable StoragePlugins bootstrapPlugins) {
+// if bootstrapPlugins is not null -- fresh Drill set up
+StoragePlugins pluginsToBeWrittenToPersistentStore;
+
+StoragePlugins newPlugins = getNewStoragePlugins();
+
+if (newPlugins != null) {
+ pluginsToBeWrittenToPersistentStore = new StoragePlugins(new
HashMap<>());
+ Optional.ofNullable(bootstrapPlugins)
+ .ifPresent(pluginsToBeWrittenToPersistentStore::putAll);
+
+ for (Map.Entry newPlugin : newPlugins) {
+String pluginName = newPlugin.getKey();
+StoragePluginConfig oldPluginConfig =
Optional.ofNullable(bootstrapPlugins)
+.map(plugins -> plugins.getConfig(pluginName))
+.orElse(persistentStore.get(pluginName));
+StoragePluginConfig updatedStatusPluginConfig =
updatePluginStatus(oldPluginConfig, newPlugin.getValue());
+pluginsToBeWrittenToPersistentStore.put(pluginName,
updatedStatusPluginConfig);
+ }
+} else {
+ pluginsToBeWrittenToPersistentStore = bootstrapPlugins;
+}
+
+// load pluginsToBeWrittenToPersistentStore to Persistent Store
+Optional.ofNullable(pluginsToBeWrittenToPersistentStore)
+.ifPresent(plugins -> plugins.forEach(plugin ->
persistentStore.put(plugin.getKey(), plugin.getValue(;
+
+if (newPlugins != null) {
+ PluginsOverrideFileAction finalActionOnPluginsOverrideFile =
+
PluginsOverrideFileAction.valueOf(context.getConfig().getString(ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE).toUpperCase());
+ switch (finalActionOnPluginsOverrideFile) {
+case RENAME:
+
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199785168
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandlerService.java
##
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import static
org.apache.drill.exec.store.StoragePluginRegistry.ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF} conf file
+ *
+ * TODO: DRILL-6564: It can be improved with configs versioning and service of
creating
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
+ */
+public class StoragePluginsHandlerService implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsHandlerService.class);
+
+ private final LogicalPlanPersistence hoconLogicalPlanPersistence;
+ private final DrillbitContext context;
+ private URL pluginsOverrideFileUrl;
+
+ public StoragePluginsHandlerService(DrillbitContext context) {
+this.context = context;
+this.hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public void loadPlugins(@NotNull PersistentStore
persistentStore,
+ @Nullable StoragePlugins bootstrapPlugins) {
+// if bootstrapPlugins is not null -- fresh Drill set up
+StoragePlugins pluginsToBeWrittenToPersistentStore;
+
+StoragePlugins newPlugins = getNewStoragePlugins();
+
+if (newPlugins != null) {
+ pluginsToBeWrittenToPersistentStore = new StoragePlugins(new
HashMap<>());
+ Optional.ofNullable(bootstrapPlugins)
+ .ifPresent(pluginsToBeWrittenToPersistentStore::putAll);
+
+ for (Map.Entry newPlugin : newPlugins) {
+String pluginName = newPlugin.getKey();
+StoragePluginConfig oldPluginConfig =
Optional.ofNullable(bootstrapPlugins)
+.map(plugins -> plugins.getConfig(pluginName))
+.orElse(persistentStore.get(pluginName));
+StoragePluginConfig updatedStatusPluginConfig =
updatePluginStatus(oldPluginConfig, newPlugin.getValue());
+pluginsToBeWrittenToPersistentStore.put(pluginName,
updatedStatusPluginConfig);
+ }
+} else {
+ pluginsToBeWrittenToPersistentStore = bootstrapPlugins;
+}
+
+// load pluginsToBeWrittenToPersistentStore to Persistent Store
+Optional.ofNullable(pluginsToBeWrittenToPersistentStore)
+.ifPresent(plugins -> plugins.forEach(plugin ->
persistentStore.put(plugin.getKey(), plugin.getValue(;
+
+if (newPlugins != null) {
+ PluginsOverrideFileAction finalActionOnPluginsOverrideFile =
+
PluginsOverrideFileAction.valueOf(context.getConfig().getString(ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE).toUpperCase());
+ switch (finalActionOnPluginsOverrideFile) {
+case RENAME:
+
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199750865
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandlerService.java
##
@@ -0,0 +1,197 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import static
org.apache.drill.exec.store.StoragePluginRegistry.ACTION_ON_STORAGE_PLUGINS_OVERRIDE_FILE;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF} conf file
+ *
+ * TODO: DRILL-6564: It can be improved with configs versioning and service of
creating
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
+ */
+public class StoragePluginsHandlerService implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsHandlerService.class);
+
+ private final LogicalPlanPersistence hoconLogicalPlanPersistence;
+ private final DrillbitContext context;
+ private URL pluginsOverrideFileUrl;
+
+ public StoragePluginsHandlerService(DrillbitContext context) {
+this.context = context;
+this.hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public void loadPlugins(@NotNull PersistentStore
persistentStore,
+ @Nullable StoragePlugins bootstrapPlugins) {
+// if bootstrapPlugins is not null -- fresh Drill set up
+StoragePlugins pluginsToBeWrittenToPersistentStore;
+
+StoragePlugins newPlugins = getNewStoragePlugins();
+
+if (newPlugins != null) {
+ pluginsToBeWrittenToPersistentStore = new StoragePlugins(new
HashMap<>());
+ Optional.ofNullable(bootstrapPlugins)
+ .ifPresent(pluginsToBeWrittenToPersistentStore::putAll);
+
+ for (Map.Entry newPlugin : newPlugins) {
+String pluginName = newPlugin.getKey();
+StoragePluginConfig oldPluginConfig =
Optional.ofNullable(bootstrapPlugins)
+.map(plugins -> plugins.getConfig(pluginName))
+.orElse(persistentStore.get(pluginName));
+StoragePluginConfig updatedStatusPluginConfig =
updatePluginStatus(oldPluginConfig, newPlugin.getValue());
+pluginsToBeWrittenToPersistentStore.put(pluginName,
updatedStatusPluginConfig);
+ }
+} else {
+ pluginsToBeWrittenToPersistentStore = bootstrapPlugins;
+}
+
+// load pluginsToBeWrittenToPersistentStore to Persistent Store
+Optional.ofNullable(pluginsToBeWrittenToPersistentStore)
+.ifPresent(plugins -> plugins.forEach(plugin ->
persistentStore.put(plugin.getKey(), plugin.getValue(;
+
+if (newPlugins != null) {
+ PluginsOverrideFileAction finalActionOnPluginsOverrideFile =
Review comment:
-> `overrideFileAction`
Done.
This is an automated message from the Apache Git Service.
To respond to the message, please log on
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199358015
##
File path:
contrib/storage-kafka/src/main/resources/bootstrap-storage-plugins.json
##
@@ -2,8 +2,8 @@
"storage":{
kafka : {
type:"kafka",
- enabled: false,
- kafkaConsumerProps: {"bootstrap.servers":"localhost:9092", "group.id" :
"drill-consumer"}
+ kafkaConsumerProps: {"bootstrap.servers":"localhost:9092", "group.id" :
"drill-consumer"},
+ enabled: false
Review comment:
It looks like Hive plugin is the only one plugin with such order of
properties. For all other plugins the enabled status appears in the end of
config after deserializing.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199358015
##
File path:
contrib/storage-kafka/src/main/resources/bootstrap-storage-plugins.json
##
@@ -2,8 +2,8 @@
"storage":{
kafka : {
type:"kafka",
- enabled: false,
- kafkaConsumerProps: {"bootstrap.servers":"localhost:9092", "group.id" :
"drill-consumer"}
+ kafkaConsumerProps: {"bootstrap.servers":"localhost:9092", "group.id" :
"drill-consumer"},
+ enabled: false
Review comment:
It looks like Hive plugin is the only one plugin with such order of
properties. For all other plugins the enabled status appears in the end of
config after deserializing.
So I return enabled status for Hive plugin and leave for others.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199359888
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandlerService.java
##
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+import java.io.IOException;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF} conf file
+ *
+ * TODO: DRILL-6564: It can be improved with configs versioning and service of
creating
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
+ */
+public class StoragePluginsHandlerService implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsHandlerService.class);
+
+ private final LogicalPlanPersistence hoconLogicalPlanPersistence;
+
+ public StoragePluginsHandlerService(DrillbitContext context) {
+hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public void loadPlugins(@NotNull PersistentStore
persistentStore,
+ @Nullable StoragePlugins bootstrapPlugins) {
+// if bootstrapPlugins is not null -- fresh Drill set up
+StoragePlugins pluginsToBeWrittenToPersistentStore;
+
+StoragePlugins newPlugins = getNewStoragePlugins();
+
+if (newPlugins != null) {
+ pluginsToBeWrittenToPersistentStore = new StoragePlugins(new
HashMap<>());
+ Optional.ofNullable(bootstrapPlugins)
+ .ifPresent(pluginsToBeWrittenToPersistentStore::putAll);
+
+ for (Map.Entry newPlugin : newPlugins) {
+String pluginName = newPlugin.getKey();
+StoragePluginConfig oldPluginConfig =
Optional.ofNullable(bootstrapPlugins)
+.map(plugins -> plugins.getConfig(pluginName))
+.orElse(persistentStore.get(pluginName));
+StoragePluginConfig updatedStatusPluginConfig =
updatePluginStatus(oldPluginConfig, newPlugin.getValue());
+pluginsToBeWrittenToPersistentStore.put(pluginName,
updatedStatusPluginConfig);
+ }
+} else {
+ pluginsToBeWrittenToPersistentStore = bootstrapPlugins;
+}
+
+// load pluginsToBeWrittenToPersistentStore to Persistent Store
+Optional.ofNullable(pluginsToBeWrittenToPersistentStore)
+.ifPresent(plugins -> plugins.forEach(plugin ->
persistentStore.put(plugin.getKey(), plugin.getValue(;
+ }
+
+ /**
+ * Helper method to identify the enabled status for new storage plugins
config. If this status is absent in the updater
+ * file, the status is kept from the configs, which are going to be updated
+ *
+ * @param oldPluginConfig current storage plugin config from Persistent
Store or bootstrap config file
+ * @param newPluginConfig new storage plugin config
+ * @return new storage plugin config with updated enabled status
+ */
+ private StoragePluginConfig updatePluginStatus(@Nullable StoragePluginConfig
oldPluginConfig,
+ @NotNull StoragePluginConfig
newPluginConfig) {
+if
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199359807
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandler.java
##
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+
+/**
+ * Storage plugins handler is an additional service for updating storage
plugins configs from the file
+ */
+public interface StoragePluginsHandler {
+
+ /**
+ * Update incoming storage plugins configs from persistence store if
present, otherwise bootstrap plugins configs.
+ * One of the params should be null, second shouldn't
+ *
+ * @param persistentStore the last storage plugins configs from persistence
store
+ * @param bootstrapPlugins bootstrap storage plugins, which are used in case
of first Drill start up
+ * @return all storage plugins, which should be loaded into persistence store
+ */
+ void loadPlugins(PersistentStore persistentStore,
StoragePlugins bootstrapPlugins);
Review comment:
Let's say the current approach is method1 and the approach of transferring
the all configs from PS to handler is method2.
So for both of them the first stage is common:
1. Getting the iterator and check for any element -
`pluginSystemTable.getAll().hasNext()`, which is used to detect whether any
plugins are present in PStore or this is the first set-up.
2. Then for method 2 I need to extract every plugin configs and put it to
the `StoragePlugins` - N calls to PStore.
For method 1 I need pass just reference to PStore.
3. For method2 I need to update plugins and to determine only updated
plugins configs to put them to PStore - <= N calls.
Similar in method1 I need to get new plugins if exist and put them to PStore
- <= N calls.
_
The benefit in the second stage.
I agree with you regarding registry responsibilities. But current approach
is also good: anyway currently `StoragePluginsHandler` instance is a part of
`StoragePluginRegistryImpl` and also loading configs is better from code
understanding point of view than updating and returning different results for
different modes (you can compare with changes from the first commit).
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199358139
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/NamedStoragePluginConfig.java
##
@@ -17,22 +17,51 @@
*/
package org.apache.drill.exec.store;
+import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.drill.common.logical.StoragePluginConfig;
import com.fasterxml.jackson.annotation.JsonTypeName;
-@JsonTypeName("named")
+@JsonTypeName(NamedStoragePluginConfig.NAME)
public class NamedStoragePluginConfig extends StoragePluginConfig {
- public String name;
+
+ public static final String NAME = "named";
+
+ private final String name;
+
+ public NamedStoragePluginConfig(@JsonProperty("name") String name) {
Review comment:
Missed that, thanks. Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199357677
##
File path:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
##
@@ -52,18 +50,18 @@
public class HiveSchemaFactory implements SchemaFactory {
static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(HiveSchemaFactory.class);
- // MetaStoreClient created using process user credentials
- private final DrillHiveMetaStoreClient processUserMetastoreClient;
- // MetasStoreClient created using SchemaConfig credentials
- private final LoadingCache
metaStoreClientLoadingCache;
+ // MetaStoreClient created using process user credentials. Null if client
can't be instantiated
+ private DrillHiveMetaStoreClient processUserMetastoreClient;
Review comment:
It may not be initialized in the constructor. It was done to avoid `null`
value in Hive storage plugin update window.
But I don't like this approach anymore.
I have described the issue here:
[DRILL-6412](https://issues.apache.org/jira/browse/DRILL-6412?focusedCommentId=16528944=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16528944).
I think until it will be solved as mentioned in DRILL-6412, the workaround
could be used,: `"hive.metastore.schema.verification": "false"` property in
Hive `bootstrap-storage.json`. It allows instantiate Hive client properly as in
earlier 1.2 version of Drill Hive client.
Also this properties should be documented for configuring Hive Embedded
Metastore:
https://drill.apache.org/docs/hive-storage-plugin/#hive-embedded-metastore-configuration
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199308452
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandler.java
##
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+
+/**
+ * Storage plugins handler is an additional service for updating storage
plugins configs from the file
+ */
+public interface StoragePluginsHandler {
+
+ /**
+ * Update incoming storage plugins configs from persistence store if
present, otherwise bootstrap plugins configs.
+ * One of the params should be null, second shouldn't
+ *
+ * @param persistentStore the last storage plugins configs from persistence
store
+ * @param bootstrapPlugins bootstrap storage plugins, which are used in case
of first Drill start up
+ * @return all storage plugins, which should be loaded into persistence store
+ */
+ void loadPlugins(PersistentStore persistentStore,
StoragePlugins bootstrapPlugins);
Review comment:
I thought about it. There was some difficulties with it:
* One - we need to do more calls to PS. We need to get all plugins from PS
with iterator one by one.
In my case we even don't get plugins (if `enabled` status is defined in
the new plugin).
* Arina proposed to determine handler not just for updating, but for loading
plugins configs too. In this case it is more independent and the logic became
simpler (annotations show nullability of variables)
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199308790
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandlerService.java
##
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillRuntimeException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.annotation.Nullable;
+import javax.validation.constraints.NotNull;
+import java.io.IOException;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF} conf file
+ *
+ * TODO: DRILL-6564: It can be improved with configs versioning and service of
creating
+ * {@link CommonConstants#STORAGE_PLUGINS_OVERRIDE_CONF}
+ */
+public class StoragePluginsHandlerService implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsHandlerService.class);
+
+ private final LogicalPlanPersistence hoconLogicalPlanPersistence;
+
+ public StoragePluginsHandlerService(DrillbitContext context) {
+hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public void loadPlugins(@NotNull PersistentStore
persistentStore,
+ @Nullable StoragePlugins bootstrapPlugins) {
+// if bootstrapPlugins is not null -- fresh Drill set up
+StoragePlugins pluginsToBeWrittenToPersistentStore;
+
+StoragePlugins newPlugins = getNewStoragePlugins();
+
+if (newPlugins != null) {
+ pluginsToBeWrittenToPersistentStore = new StoragePlugins(new
HashMap<>());
+ Optional.ofNullable(bootstrapPlugins)
+ .ifPresent(pluginsToBeWrittenToPersistentStore::putAll);
+
+ for (Map.Entry newPlugin : newPlugins) {
+String pluginName = newPlugin.getKey();
+StoragePluginConfig oldPluginConfig =
Optional.ofNullable(bootstrapPlugins)
+.map(plugins -> plugins.getConfig(pluginName))
+.orElse(persistentStore.get(pluginName));
+StoragePluginConfig updatedStatusPluginConfig =
updatePluginStatus(oldPluginConfig, newPlugin.getValue());
+pluginsToBeWrittenToPersistentStore.put(pluginName,
updatedStatusPluginConfig);
+ }
+} else {
+ pluginsToBeWrittenToPersistentStore = bootstrapPlugins;
+}
+
+// load pluginsToBeWrittenToPersistentStore to Persistent Store
+Optional.ofNullable(pluginsToBeWrittenToPersistentStore)
+.ifPresent(plugins -> plugins.forEach(plugin ->
persistentStore.put(plugin.getKey(), plugin.getValue(;
+ }
+
+ /**
+ * Helper method to identify the enabled status for new storage plugins
config. If this status is absent in the updater
+ * file, the status is kept from the configs, which are going to be updated
+ *
+ * @param oldPluginConfig current storage plugin config from Persistent
Store or bootstrap config file
+ * @param newPluginConfig new storage plugin config
+ * @return new storage plugin config with updated enabled status
+ */
+ private StoragePluginConfig updatePluginStatus(@Nullable StoragePluginConfig
oldPluginConfig,
+ @NotNull StoragePluginConfig
newPluginConfig) {
+if
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199307744
##
File path:
contrib/storage-kafka/src/main/resources/bootstrap-storage-plugins.json
##
@@ -2,8 +2,8 @@
"storage":{
kafka : {
type:"kafka",
- enabled: false,
- kafkaConsumerProps: {"bootstrap.servers":"localhost:9092", "group.id" :
"drill-consumer"}
+ kafkaConsumerProps: {"bootstrap.servers":"localhost:9092", "group.id" :
"drill-consumer"},
+ enabled: false
Review comment:
Currently after deserializing all the plugins configs are shown in Drill UI
with enabled status in the end.
It is replaced in config files too for consistency.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199310178
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
##
@@ -121,69 +121,73 @@ public StoragePlugin load(StoragePluginConfig config)
throws Exception {
}
@Override
- public void init() throws DrillbitStartupException {
+ public void init() {
availablePlugins = findAvailablePlugins(classpathScan);
+try {
+ StoragePlugins bootstrapPlugins = pluginSystemTable.getAll().hasNext() ?
null : loadBootstrapPlugins();
-// create registered plugins defined in "storage-plugins.json"
-plugins.putAll(createPlugins());
+ StoragePluginsHandler storagePluginsUpdaterService = new
StoragePluginsHandlerService(context);
+ storagePluginsUpdaterService.loadPlugins(pluginSystemTable,
bootstrapPlugins);
+
+ // Defines enabled plugins
+ defineEnabledPlugins();
+} catch (IOException e) {
+ logger.error("Failure setting up storage enabledPlugins. Drillbit
exiting.", e);
+ throw new IllegalStateException(e);
+}
}
- @SuppressWarnings("resource")
- private Map createPlugins() throws
DrillbitStartupException {
-try {
- /*
- * Check if the storage plugins system table has any entries. If not,
load the boostrap-storage-plugin file into
- * the system table.
- */
- if (!pluginSystemTable.getAll().hasNext()) {
-// bootstrap load the config since no plugins are stored.
-logger.info("No storage plugin instances configured in persistent
store, loading bootstrap configuration.");
-Collection urls =
ClassPathScanner.forResource(ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE,
false);
-if (urls != null && !urls.isEmpty()) {
- logger.info("Loading the storage plugin configs from URLs {}.",
urls);
- Map pluginURLMap = Maps.newHashMap();
- for (URL url : urls) {
-String pluginsData = Resources.toString(url, Charsets.UTF_8);
-StoragePlugins plugins =
lpPersistence.getMapper().readValue(pluginsData, StoragePlugins.class);
-for (Map.Entry config : plugins) {
- if (!definePluginConfig(config.getKey(), config.getValue())) {
-logger.warn("Duplicate plugin instance '{}' defined in [{},
{}], ignoring the later one.",
-config.getKey(), pluginURLMap.get(config.getKey()), url);
-continue;
- }
- pluginURLMap.put(config.getKey(), url);
-}
- }
-} else {
- throw new IOException("Failure finding " +
ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE);
-}
+ /**
+ * Read bootstrap storage plugins {@link
ExecConstants#BOOTSTRAP_STORAGE_PLUGINS_FILE} files for the first fresh
+ * instantiating of Drill
+ *
+ * @return bootstrap storage plugins
+ * @throws IOException if a read error occurs
+ */
+ private StoragePlugins loadBootstrapPlugins() throws IOException {
+// bootstrap load the config since no plugins are stored.
+logger.info("No storage plugin instances configured in persistent store,
loading bootstrap configuration.");
+Set urls =
ClassPathScanner.forResource(ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE,
false);
+if (urls != null && !urls.isEmpty()) {
+ logger.info("Loading the storage plugin configs from URLs {}.", urls);
+ StoragePlugins bootstrapPlugins = new StoragePlugins(new HashMap<>());
+ for (URL url : urls) {
+String pluginsData = Resources.toString(url, Charsets.UTF_8);
+
bootstrapPlugins.putAll(lpPersistence.getMapper().readValue(pluginsData,
StoragePlugins.class));
Review comment:
Agree. Simpler doesn't mean better.
Returned the previous logic.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199276235
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsUpdater.java
##
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillIOException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.validation.constraints.NotNull;
+import java.io.IOException;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_UPDATER_FILE} conf file
+ *
+ * TODO: it can be improved with configs versioning and service of creating
{@link CommonConstants#STORAGE_PLUGINS_UPDATER_FILE}
+ */
+public class StoragePluginsUpdater implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsUpdater.class);
+
+ final DrillbitContext context;
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r198884749
##
File path: common/src/main/java/org/apache/drill/exec/metrics/DrillMetrics.java
##
@@ -36,11 +36,11 @@
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillMetrics.class);
public static final boolean METRICS_JMX_OUTPUT_ENABLED =
- SystemPropertyUtil.getBoolean("drill.metrics.jmx.enabled", true);
+ SystemPropertyUtil.getBoolean("drill.exec.metrics.jmx.enabled", true);
Review comment:
The issues comes from `drill-override-example.conf`:
https://github.com/apache/drill/blob/master/distribution/src/resources/drill-override-example.conf#L22
https://github.com/apache/drill/blob/master/distribution/src/resources/drill-override-example.conf#L66
Since drill.apache.org tells the options without `exec`
[link](https://drill.apache.org/docs/monitoring-metrics/#disabling-drill-metrics)
I reverted the constants names and changed `drill-override-example.conf`
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199073387
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
##
@@ -194,25 +215,21 @@ public void init() throws DrillbitStartupException {
* @param config plugin config
* @param plugin plugin implementation
*/
-
- public void definePlugin(String name, StoragePluginConfig config,
StoragePlugin plugin) {
-addPlugin(name, plugin);
-definePluginConfig(name, config);
- }
-
- private boolean definePluginConfig(String name, StoragePluginConfig config) {
-return pluginSystemTable.putIfAbsent(name, config);
+ @VisibleForTesting
+ public void addPluginTonPersistenceStoreIfAbsent(String name,
StoragePluginConfig config, StoragePlugin plugin) {
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r198892158
##
File path: distribution/src/resources/drill-override-example.conf
##
@@ -251,6 +251,9 @@ drill.exec: {
protocol: "TLSv1.2",
#ssl provider. May be "JDK" or "OPENSSL". Default is "JDK"
provider: "JDK"
+ },
+ storage_plugins.update: {
Review comment:
The option is removed
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r198932079
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
##
@@ -121,71 +117,96 @@ public StoragePlugin load(StoragePluginConfig config)
throws Exception {
}
@Override
- public void init() throws DrillbitStartupException {
+ public void init() {
availablePlugins = findAvailablePlugins(classpathScan);
-// create registered plugins defined in "storage-plugins.json"
-plugins.putAll(createPlugins());
- }
-
- @SuppressWarnings("resource")
- private Map createPlugins() throws
DrillbitStartupException {
try {
- /*
- * Check if the storage plugins system table has any entries. If not,
load the boostrap-storage-plugin file into
- * the system table.
- */
+ StoragePlugins pluginsToBeWrittenToPersistenceStore = null;
+ final boolean isPluginsUpdaterEnabled =
context.getConfig().getBoolean(ExecConstants.STORAGE_PLUGINS_UPDATE);
Review comment:
the option is removed
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r198883586
##
File path:
common/src/main/java/org/apache/drill/common/exceptions/DrillIOException.java
##
@@ -19,24 +19,23 @@
import java.io.IOException;
-public class DrillIOException extends IOException{
+public class DrillIOException extends IOException {
static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(DrillIOException.class);
public DrillIOException() {
super();
}
- public DrillIOException(String message, Throwable cause) {
-super(message, cause);
+ public DrillIOException(Throwable cause, String formatMessage,
Object...args) {
Review comment:
It makes sense. Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199073178
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
##
@@ -63,17 +65,15 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.cache.RemovalListener;
-import com.google.common.cache.RemovalNotification;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
import com.google.common.io.Resources;
public class StoragePluginRegistryImpl implements StoragePluginRegistry {
private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginRegistryImpl.class);
+ /** Drill supported plugins */
private Map> availablePlugins =
Collections.emptyMap();
- private final StoragePluginMap plugins = new StoragePluginMap();
+ /** Enabled plugins */
Review comment:
This is not a comment, It is one line JavaDoc for the variable.
I have changed format to the three lines Java doc
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199073307
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
##
@@ -121,71 +117,96 @@ public StoragePlugin load(StoragePluginConfig config)
throws Exception {
}
@Override
- public void init() throws DrillbitStartupException {
+ public void init() {
availablePlugins = findAvailablePlugins(classpathScan);
-// create registered plugins defined in "storage-plugins.json"
-plugins.putAll(createPlugins());
- }
-
- @SuppressWarnings("resource")
- private Map createPlugins() throws
DrillbitStartupException {
try {
- /*
- * Check if the storage plugins system table has any entries. If not,
load the boostrap-storage-plugin file into
- * the system table.
- */
+ StoragePlugins pluginsToBeWrittenToPersistenceStore = null;
+ final boolean isPluginsUpdaterEnabled =
context.getConfig().getBoolean(ExecConstants.STORAGE_PLUGINS_UPDATE);
if (!pluginSystemTable.getAll().hasNext()) {
-// bootstrap load the config since no plugins are stored.
-logger.info("No storage plugin instances configured in persistent
store, loading bootstrap configuration.");
-Collection urls =
ClassPathScanner.forResource(ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE,
false);
-if (urls != null && !urls.isEmpty()) {
- logger.info("Loading the storage plugin configs from URLs {}.",
urls);
- Map pluginURLMap = Maps.newHashMap();
- for (URL url : urls) {
-String pluginsData = Resources.toString(url, Charsets.UTF_8);
-StoragePlugins plugins =
lpPersistence.getMapper().readValue(pluginsData, StoragePlugins.class);
-for (Map.Entry config : plugins) {
- if (!definePluginConfig(config.getKey(), config.getValue())) {
-logger.warn("Duplicate plugin instance '{}' defined in [{},
{}], ignoring the later one.",
-config.getKey(), pluginURLMap.get(config.getKey()), url);
-continue;
- }
- pluginURLMap.put(config.getKey(), url);
-}
- }
-} else {
- throw new IOException("Failure finding " +
ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE);
+// create registered plugins defined in "storage-plugins.json"
+StoragePlugins bootstrapPlugins = readBootstrapStoragePlugins();
+if (isPluginsUpdaterEnabled) {
+ StoragePluginsHandler storagePluginsUpdaterService = new
StoragePluginsUpdater(context);
+ pluginsToBeWrittenToPersistenceStore =
storagePluginsUpdaterService.updatePlugins(bootstrapPlugins, null);
+}
+if (pluginsToBeWrittenToPersistenceStore == null) {
+ pluginsToBeWrittenToPersistenceStore = bootstrapPlugins;
+}
+ } else {
+if (isPluginsUpdaterEnabled) {
+ StoragePluginsHandler storagePluginsUpdaterService = new
StoragePluginsUpdater(context);
+ pluginsToBeWrittenToPersistenceStore =
storagePluginsUpdaterService.updatePlugins(null, pluginSystemTable);
}
}
- Map activePlugins = new HashMap();
- for (Map.Entry entry :
Lists.newArrayList(pluginSystemTable.getAll())) {
-String name = entry.getKey();
-StoragePluginConfig config = entry.getValue();
-if (config.isEnabled()) {
- try {
-StoragePlugin plugin = create(name, config);
-activePlugins.put(name, plugin);
- } catch (ExecutionSetupException e) {
-logger.error("Failure while setting up StoragePlugin with name:
'{}', disabling.", name, e);
-config.setEnabled(false);
-pluginSystemTable.put(name, config);
- }
+ // load plugins to persistence store
+ if (pluginsToBeWrittenToPersistenceStore != null) {
+for (Entry plugin :
pluginsToBeWrittenToPersistenceStore) {
+ pluginSystemTable.put(plugin.getKey(), plugin.getValue());
}
}
- activePlugins.put(INFORMATION_SCHEMA_PLUGIN, new
InfoSchemaStoragePlugin(new InfoSchemaConfig(), context,
- INFORMATION_SCHEMA_PLUGIN));
- activePlugins.put(SYS_PLUGIN, new
SystemTablePlugin(SystemTablePluginConfig.INSTANCE, context, SYS_PLUGIN));
-
- return activePlugins;
+ defineEnabledPlugins();
} catch (IOException e) {
- logger.error("Failure setting up storage plugins. Drillbit exiting.",
e);
+ logger.error("Failure setting up storage enabledPlugins. Drillbit
exiting.", e);
throw new IllegalStateException(e);
}
}
+ /**
+ * Read bootstrap storage plugins {@link
ExecConstants#BOOTSTRAP_STORAGE_PLUGINS_FILE} files for the first fresh
+ * instantiating of Drill
+ *
+ * @return bootstrap storage plugins
+ * @throws
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199075637
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsHandler.java
##
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+
+/**
+ * Storage plugins handler is an additional service for updating storage
plugins configs from the file
+ */
+public interface StoragePluginsHandler {
+
+ /**
+ * Update incoming storage plugins configs from persistence store if
present, otherwise bootstrap plugins configs.
+ * One of the params should be null, second shouldn't
+ *
+ * @param bootstrapPlugins bootstrap storage plugins, which are used in case
of first Drill start up
+ * @param persistentStore the last storage plugins configs from persistence
store
+ * @return all storage plugins, which should be loaded into persistence store
+ */
+ StoragePlugins updatePlugins(StoragePlugins bootstrapPlugins,
PersistentStore persistentStore);
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler URL: https://github.com/apache/drill/pull/1345#discussion_r199079357 ## File path: distribution/src/deb/control/conffiles ## @@ -16,3 +16,4 @@ /etc/drill/conf/drill-override.conf /etc/drill/conf/logback.xml /etc/drill/conf/drill-env.sh +/etc/drill/conf/storage-plugins-updates.conf Review comment: It should be removed. Thanks for catching this. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199076849
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsUpdater.java
##
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillIOException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.validation.constraints.NotNull;
+import java.io.IOException;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_UPDATER_FILE} conf file
+ *
+ * TODO: it can be improved with configs versioning and service of creating
{@link CommonConstants#STORAGE_PLUGINS_UPDATER_FILE}
+ */
+public class StoragePluginsUpdater implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsUpdater.class);
+
+ final DrillbitContext context;
+ final LogicalPlanPersistence hoconLogicalPlanPersistence;
+
+ public StoragePluginsUpdater(DrillbitContext context) {
+this.context = context;
+hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public StoragePlugins updatePlugins(StoragePlugins bootstrapPlugins,
PersistentStore persistentStore) {
+StoragePlugins plugins = null;
+Set urlSet =
ClassPathScanner.forResource(CommonConstants.STORAGE_PLUGINS_UPDATER_FILE,
false);
+try {
+ if (!urlSet.isEmpty()) {
+if (urlSet.size() != 1) {
+ throw new DrillIOException("More than one %s file is placed in
Drill's classpath",
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199073271
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
##
@@ -121,71 +117,96 @@ public StoragePlugin load(StoragePluginConfig config)
throws Exception {
}
@Override
- public void init() throws DrillbitStartupException {
+ public void init() {
availablePlugins = findAvailablePlugins(classpathScan);
-// create registered plugins defined in "storage-plugins.json"
-plugins.putAll(createPlugins());
- }
-
- @SuppressWarnings("resource")
- private Map createPlugins() throws
DrillbitStartupException {
try {
- /*
- * Check if the storage plugins system table has any entries. If not,
load the boostrap-storage-plugin file into
- * the system table.
- */
+ StoragePlugins pluginsToBeWrittenToPersistenceStore = null;
+ final boolean isPluginsUpdaterEnabled =
context.getConfig().getBoolean(ExecConstants.STORAGE_PLUGINS_UPDATE);
if (!pluginSystemTable.getAll().hasNext()) {
-// bootstrap load the config since no plugins are stored.
-logger.info("No storage plugin instances configured in persistent
store, loading bootstrap configuration.");
-Collection urls =
ClassPathScanner.forResource(ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE,
false);
-if (urls != null && !urls.isEmpty()) {
- logger.info("Loading the storage plugin configs from URLs {}.",
urls);
- Map pluginURLMap = Maps.newHashMap();
- for (URL url : urls) {
-String pluginsData = Resources.toString(url, Charsets.UTF_8);
-StoragePlugins plugins =
lpPersistence.getMapper().readValue(pluginsData, StoragePlugins.class);
-for (Map.Entry config : plugins) {
- if (!definePluginConfig(config.getKey(), config.getValue())) {
-logger.warn("Duplicate plugin instance '{}' defined in [{},
{}], ignoring the later one.",
-config.getKey(), pluginURLMap.get(config.getKey()), url);
-continue;
- }
- pluginURLMap.put(config.getKey(), url);
-}
- }
-} else {
- throw new IOException("Failure finding " +
ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE);
+// create registered plugins defined in "storage-plugins.json"
+StoragePlugins bootstrapPlugins = readBootstrapStoragePlugins();
+if (isPluginsUpdaterEnabled) {
+ StoragePluginsHandler storagePluginsUpdaterService = new
StoragePluginsUpdater(context);
+ pluginsToBeWrittenToPersistenceStore =
storagePluginsUpdaterService.updatePlugins(bootstrapPlugins, null);
+}
+if (pluginsToBeWrittenToPersistenceStore == null) {
+ pluginsToBeWrittenToPersistenceStore = bootstrapPlugins;
+}
+ } else {
+if (isPluginsUpdaterEnabled) {
+ StoragePluginsHandler storagePluginsUpdaterService = new
StoragePluginsUpdater(context);
+ pluginsToBeWrittenToPersistenceStore =
storagePluginsUpdaterService.updatePlugins(null, pluginSystemTable);
}
}
- Map activePlugins = new HashMap();
- for (Map.Entry entry :
Lists.newArrayList(pluginSystemTable.getAll())) {
-String name = entry.getKey();
-StoragePluginConfig config = entry.getValue();
-if (config.isEnabled()) {
- try {
-StoragePlugin plugin = create(name, config);
-activePlugins.put(name, plugin);
- } catch (ExecutionSetupException e) {
-logger.error("Failure while setting up StoragePlugin with name:
'{}', disabling.", name, e);
-config.setEnabled(false);
-pluginSystemTable.put(name, config);
- }
+ // load plugins to persistence store
Review comment:
done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199276413
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsUpdater.java
##
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillIOException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.validation.constraints.NotNull;
+import java.io.IOException;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_UPDATER_FILE} conf file
+ *
+ * TODO: it can be improved with configs versioning and service of creating
{@link CommonConstants#STORAGE_PLUGINS_UPDATER_FILE}
+ */
+public class StoragePluginsUpdater implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsUpdater.class);
+
+ final DrillbitContext context;
+ final LogicalPlanPersistence hoconLogicalPlanPersistence;
+
+ public StoragePluginsUpdater(DrillbitContext context) {
+this.context = context;
+hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public StoragePlugins updatePlugins(StoragePlugins bootstrapPlugins,
PersistentStore persistentStore) {
+StoragePlugins plugins = null;
+Set urlSet =
ClassPathScanner.forResource(CommonConstants.STORAGE_PLUGINS_UPDATER_FILE,
false);
+try {
+ if (!urlSet.isEmpty()) {
+if (urlSet.size() != 1) {
+ throw new DrillIOException("More than one %s file is placed in
Drill's classpath",
+ CommonConstants.STORAGE_PLUGINS_UPDATER_FILE);
+}
+URL fileUrl = urlSet.iterator().next();
+plugins = new StoragePlugins(new HashMap<>());
+if (bootstrapPlugins != null) {
+ plugins.putAll(bootstrapPlugins);
+}
+
+String newPluginsData = Resources.toString(fileUrl, Charsets.UTF_8);
+StoragePlugins newPlugins =
hoconLogicalPlanPersistence.getMapper().readValue(newPluginsData,
StoragePlugins.class);
+
+for (Map.Entry newPlugin : newPlugins) {
+ String pluginName = newPlugin.getKey();
+ StoragePluginConfig oldPluginConfig = persistentStore == null ?
bootstrapPlugins.getConfig(pluginName) :
+ persistentStore.get(pluginName);
+ StoragePluginConfig updatedStatusPluginConfig =
+ determineUndefinedEnabledStatus(oldPluginConfig,
newPlugin.getValue());
+ plugins.put(pluginName, updatedStatusPluginConfig);
+}
+ }
+} catch (IOException e) {
+ logger.error("Failures are obtained while updating storage plugins from
%s file. Proceed without updating",
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r198886347
##
File path:
contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/schema/HiveSchemaFactory.java
##
@@ -72,29 +71,27 @@ public HiveSchemaFactory(final HiveStoragePlugin plugin,
final String name, fina
isDrillImpersonationEnabled =
plugin.getContext().getConfig().getBoolean(ExecConstants.IMPERSONATION_ENABLED);
try {
+ // TODO: DRILL-6412. Clients for plugin should be instantiated only for
the case, when plugin is enabled
processUserMetastoreClient =
DrillHiveMetaStoreClient.createCloseableClientWithCaching(hiveConf);
-} catch (MetaException e) {
- throw new ExecutionSetupException("Failure setting up Hive metastore
client.", e);
-}
-metaStoreClientLoadingCache = CacheBuilder
-.newBuilder()
-.expireAfterAccess(10, TimeUnit.MINUTES)
-.maximumSize(5) // Up to 5 clients for impersonation-enabled.
-.removalListener(new RemovalListener() {
- @Override
- public void onRemoval(RemovalNotification notification) {
+ metaStoreClientLoadingCache = CacheBuilder
+ .newBuilder()
+ .expireAfterAccess(10, TimeUnit.MINUTES)
+ .maximumSize(5) // Up to 5 clients for impersonation-enabled.
+ .removalListener((RemovalListener)
notification -> {
DrillHiveMetaStoreClient client = notification.getValue();
client.close();
- }
-})
-.build(new CacheLoader() {
- @Override
- public DrillHiveMetaStoreClient load(String userName) throws
Exception {
-return
DrillHiveMetaStoreClient.createClientWithAuthz(processUserMetastoreClient,
hiveConf, userName);
- }
-});
+ })
+ .build(new CacheLoader() {
+@Override
+public DrillHiveMetaStoreClient load(String userName) throws
Exception {
+ return
DrillHiveMetaStoreClient.createClientWithAuthz(processUserMetastoreClient,
hiveConf, userName);
+}
+ });
+} catch (MetaException e) {
Review comment:
If the exception is obtained the further logic from constructor doesn't make
sense.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199270039
##
File path: distribution/src/resources/storage-plugins-example.conf
##
@@ -0,0 +1,66 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This file involves storage plugins configs, which can be updated on the
Drill start-up. This is controlled with a
+# "drill.exec.storage_plugins.update.enabled" boot option, see
https://drill.apache.org/docs/start-up-options/.
+# This file is in HOCON format, see
https://github.com/typesafehub/config/blob/master/HOCON.md for more information.
+
+ "storage":{
+cp: {
+ type: "file",
+ connection: "classpath:///",
+ formats: {
+"csv" : {
+ type: "text",
+ extensions: [ "csv" ],
+ delimiter: ","
+}
+ }
+}
+ }
+ "storage":{
+dfs: {
+ type: "file",
+ enabled: false,
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199093883
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/NamedStoragePluginConfig.java
##
@@ -25,6 +25,14 @@
public class NamedStoragePluginConfig extends StoragePluginConfig {
public String name;
+ public String getName() {
+return name;
+ }
+
+ public void setName(String name) {
+this.name = name;
+ }
+
Review comment:
It is not my code, therefore I didn't pay attention to this.
I have edited these methods.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler URL: https://github.com/apache/drill/pull/1345#discussion_r199266170 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistry.java ## @@ -83,15 +84,13 @@ * @param name * @param plugin */ - void addPlugin(String name, StoragePlugin plugin); + void addPluginToEnabled(String name, StoragePlugin plugin); Review comment: It is not the same: this method adds one plugin to existing Map of enabled plugins. I suppose `addEnabledPlugin` looks good. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199276671
##
File path: exec/java-exec/src/main/resources/drill-module.conf
##
@@ -393,7 +393,11 @@ drill.exec: {
//port hunting for drillbits. Enabled only for testing purposes.
port_hunt : false,
// Allow drillbit to bind to loopback address in distributed mode. Enabled
only for testing purposes.
- allow_loopback_address_binding : false
+ allow_loopback_address_binding : false,
+ // Allow overwrite storage plugins configs from the storage-plugins.conf
file in the process of drillbit starting
+ storage_plugins.update: {
+enabled: true
Review comment:
the option is removed
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199276599
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginsUpdater.java
##
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Charsets;
+import com.google.common.io.Resources;
+import com.jasonclawson.jackson.dataformat.hocon.HoconFactory;
+import org.apache.drill.common.config.CommonConstants;
+import org.apache.drill.common.config.LogicalPlanPersistence;
+import org.apache.drill.common.exceptions.DrillIOException;
+import org.apache.drill.common.logical.StoragePluginConfig;
+import org.apache.drill.common.scanner.ClassPathScanner;
+import org.apache.drill.exec.planner.logical.StoragePlugins;
+import org.apache.drill.exec.server.DrillbitContext;
+import org.apache.drill.exec.store.sys.PersistentStore;
+
+import javax.validation.constraints.NotNull;
+import java.io.IOException;
+import java.net.URL;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Drill plugins handler, which allows to update storage plugins configs from
the
+ * {@link CommonConstants#STORAGE_PLUGINS_UPDATER_FILE} conf file
+ *
+ * TODO: it can be improved with configs versioning and service of creating
{@link CommonConstants#STORAGE_PLUGINS_UPDATER_FILE}
+ */
+public class StoragePluginsUpdater implements StoragePluginsHandler {
+ private static final org.slf4j.Logger logger =
org.slf4j.LoggerFactory.getLogger(StoragePluginsUpdater.class);
+
+ final DrillbitContext context;
+ final LogicalPlanPersistence hoconLogicalPlanPersistence;
+
+ public StoragePluginsUpdater(DrillbitContext context) {
+this.context = context;
+hoconLogicalPlanPersistence = new
LogicalPlanPersistence(context.getConfig(), context.getClasspathScan(),
+new ObjectMapper(new HoconFactory()));
+ }
+
+ @Override
+ public StoragePlugins updatePlugins(StoragePlugins bootstrapPlugins,
PersistentStore persistentStore) {
+StoragePlugins plugins = null;
+Set urlSet =
ClassPathScanner.forResource(CommonConstants.STORAGE_PLUGINS_UPDATER_FILE,
false);
+try {
+ if (!urlSet.isEmpty()) {
+if (urlSet.size() != 1) {
+ throw new DrillIOException("More than one %s file is placed in
Drill's classpath",
+ CommonConstants.STORAGE_PLUGINS_UPDATER_FILE);
+}
+URL fileUrl = urlSet.iterator().next();
+plugins = new StoragePlugins(new HashMap<>());
+if (bootstrapPlugins != null) {
+ plugins.putAll(bootstrapPlugins);
+}
+
+String newPluginsData = Resources.toString(fileUrl, Charsets.UTF_8);
+StoragePlugins newPlugins =
hoconLogicalPlanPersistence.getMapper().readValue(newPluginsData,
StoragePlugins.class);
+
+for (Map.Entry newPlugin : newPlugins) {
+ String pluginName = newPlugin.getKey();
+ StoragePluginConfig oldPluginConfig = persistentStore == null ?
bootstrapPlugins.getConfig(pluginName) :
+ persistentStore.get(pluginName);
+ StoragePluginConfig updatedStatusPluginConfig =
+ determineUndefinedEnabledStatus(oldPluginConfig,
newPlugin.getValue());
+ plugins.put(pluginName, updatedStatusPluginConfig);
+}
+ }
+} catch (IOException e) {
+ logger.error("Failures are obtained while updating storage plugins from
%s file. Proceed without updating",
+ CommonConstants.STORAGE_PLUGINS_UPDATER_FILE, e);
+ return bootstrapPlugins;
+}
+return plugins;
+ }
+
+ /**
+ * Helper method to identify the enabled status for new storage plugins
config. If this status is absent in the updater
+ * file, the status is kept from the configs, which are going to be updated
+ *
+ * @param oldPluginConfig current storage plugin config from persistence
store or bootstrap config file
+ * @param newPluginConfig new storage plugin config
+ * @return new storage plugin config with updated enabled status
+ */
+ private
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199104274
##
File path: distribution/src/resources/storage-plugins-example.conf
##
@@ -0,0 +1,66 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This file involves storage plugins configs, which can be updated on the
Drill start-up. This is controlled with a
+# "drill.exec.storage_plugins.update.enabled" boot option, see
https://drill.apache.org/docs/start-up-options/.
+# This file is in HOCON format, see
https://github.com/typesafehub/config/blob/master/HOCON.md for more information.
+
+ "storage":{
+cp: {
+ type: "file",
+ connection: "classpath:///",
+ formats: {
+"csv" : {
+ type: "text",
+ extensions: [ "csv" ],
+ delimiter: ","
+}
+ }
+}
+ }
+ "storage":{
+dfs: {
+ type: "file",
+ enabled: false,
+ connection: "hdfs:///",
+ workspaces: {
+"root": {
+ "location": "/",
+ "writable": false,
+ "defaultInputFormat": null,
+ "allowAccessOutsideWorkspace": false
+}
+ },
+ formats: {
+"parquet": {
+ "type": "parquet"
+}
+ }
+}
+ }
+ "storage":{
+mongo : {
+ type:"mongo",
+ enabled: true,
Review comment:
Actually this is the same.
https://github.com/apache/drill/blob/master/contrib/storage-mongo/src/main/resources/bootstrap-storage-plugins.json#L5
I have edited it here and in all bootstrap-storage-plugins for consistency.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler URL: https://github.com/apache/drill/pull/1345#discussion_r198882971 ## File path: common/src/main/java/org/apache/drill/common/config/CommonConstants.java ## @@ -31,4 +31,7 @@ /** Override configuration file name. (Classpath resource pathname.) */ String CONFIG_OVERRIDE_RESOURCE_PATHNAME = "drill-override.conf"; + /** Override plugins configs file name. (Classpath resource pathname.) */ + String STORAGE_PLUGINS_UPDATER_FILE = "storage-plugins.conf"; Review comment: looks good. Done 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r198931779
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java
##
@@ -248,6 +248,12 @@ private ExecConstants() {
*/
public static final String DEFAULT_TEMPORARY_WORKSPACE =
"drill.exec.default_temporary_workspace";
+ /**
+ * Storage plugins configs will be updated in the case, when
+ * {@link
org.apache.drill.common.config.CommonConstants#STORAGE_PLUGINS_UPDATER_FILE}
file is present
+ */
+ public static final String STORAGE_PLUGINS_UPDATE =
"drill.exec.storage_plugins.update.enabled";
Review comment:
The option isn't really helpful. Removed.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r198928608
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginMap.java
##
@@ -111,7 +112,7 @@ public StoragePlugin get(String name) {
return nameMap.entrySet().iterator();
}
- public Iterable names() {
+ public Set names() {
Review comment:
At least `Сollection` is needed for creating new Set from existing.
It is a requirement for plugins names, `Set` guarantees avoiding of
duplicates of names for plugins.
I have renamed it to `getNames()` and added javadoc
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199077537
##
File path:
logical/src/main/java/org/apache/drill/common/config/LogicalPlanPersistence.java
##
@@ -37,12 +37,12 @@
public class LogicalPlanPersistence {
private ObjectMapper mapper;
Review comment:
Done
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r198928698
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
##
@@ -121,71 +117,96 @@ public StoragePlugin load(StoragePluginConfig config)
throws Exception {
}
@Override
- public void init() throws DrillbitStartupException {
+ public void init() {
availablePlugins = findAvailablePlugins(classpathScan);
-// create registered plugins defined in "storage-plugins.json"
-plugins.putAll(createPlugins());
- }
-
- @SuppressWarnings("resource")
- private Map createPlugins() throws
DrillbitStartupException {
try {
- /*
- * Check if the storage plugins system table has any entries. If not,
load the boostrap-storage-plugin file into
- * the system table.
- */
+ StoragePlugins pluginsToBeWrittenToPersistenceStore = null;
Review comment:
renamed and the variable is placed in `StoragePluginsHandlerService` class
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins
Handler
URL: https://github.com/apache/drill/pull/1345#discussion_r199105919
##
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/store/NamedStoragePluginConfig.java
##
@@ -25,6 +25,14 @@
public class NamedStoragePluginConfig extends StoragePluginConfig {
public String name;
+ public String getName() {
Review comment:
Good suggestion, done.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler
vdiravka commented on a change in pull request #1345: DRILL-6494: Drill Plugins Handler URL: https://github.com/apache/drill/pull/1345#discussion_r198926537 ## File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistry.java ## @@ -83,15 +84,13 @@ * @param name Review comment: Done 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: us...@infra.apache.org With regards, Apache Git Services
