arina-ielchiieva commented on a change in pull request #1988: DRILL-7590: Refactor plugin registry URL: https://github.com/apache/drill/pull/1988#discussion_r385098807
########## File path: exec/java-exec/src/main/java/org/apache/drill/exec/store/PluginBootstrapLoaderImpl.java ########## @@ -0,0 +1,298 @@ +/* + * 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 java.io.IOException; +import java.io.InputStream; +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 org.apache.drill.common.config.ConfigConstants; +import org.apache.drill.common.config.LogicalPlanPersistence; +import org.apache.drill.common.exceptions.DrillRuntimeException; +import org.apache.drill.common.logical.FormatPluginConfig; +import org.apache.drill.common.logical.StoragePluginConfig; +import org.apache.drill.common.scanner.ClassPathScanner; +import org.apache.drill.exec.ExecConstants; +import org.apache.drill.exec.planner.logical.StoragePlugins; +import org.apache.drill.exec.store.dfs.FileSystemConfig; +import org.apache.drill.exec.util.ActionOnFile; +import org.apache.drill.shaded.guava.com.google.common.annotations.VisibleForTesting; +import org.apache.drill.shaded.guava.com.google.common.base.Charsets; +import org.apache.drill.shaded.guava.com.google.common.io.Resources; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.jasonclawson.jackson.dataformat.hocon.HoconFactory; + +/** + * Loads the set of bootstrap plugin configurations for new systems. + * Also handles upgrades to new and existing systems. Each set is given + * by a Jackson-serialized JSON file with the list of plugin (configurations). + * <p> + * The files are given via indirections in the config system to allow easier + * testing when "non-stock" configurations. + * + * <h4>Boosrtrap Files</h4> + * + * When a new system starts (boots), Drill populates the persistent store + * with a "starter" (bootstrap) set of storage plugin configurations as + * provided by each connector locator. The classic locator uses one or + * more bootsrtap files, serialized to JSON, and visible at the root of the + * class path. Each plugin implementation, if packaged in its own project, + * can provide a bootstrap file. (The core plugins share a single file.) + * + * <h4>Upgrade Files</h4> + * + * Bootstrap files populate a new system. There are times when a user upgrades + * their Drill install and we would like to push out configurations for newly + * added plugins. This is done via the optional upgrade file. + * <p> + * If the system is new, configurations from the upgrade file replace + * (override) values from the bootstrap file. The net effect, bootstrap plus + * upgrades, are written into the persistent store. + * <p> + * If the system is upgraded, then the current technique uses a highly + * unreliable system: an upgrade file exists. Any upgrades are applied on top + * of the user's stored values. The upgrade file is then deleted to avoid + * applying the same upgrades multiple times. Not idea, but it is what it + * is for now. + * + * <h4>Format Plugins</h4> + * + * Earlier versions of this code provided support for format plugins. However, + * this code never worked. The original design of Drill has format plugins + * as a set of attributes of the file system plugin config. Additional work + * is needed to allow file system configs to share plugins. Such work is a + * good idea, so the ill-fated format plugin code still appears, but does + * nothing. + * + * <h4>Config Properties</h4> + * + * <dl> + * <dt>{@code ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE} + * ({@code drill.exec.storage.bootstrap.storage})</dt> + * <dd>The name of the bootstrap file, normally + * {@code bootstrap-storage-plugins.json}.</dd> + * <dt>{@code ExecConstants.UPGRADE_STORAGE_PLUGINS_FILE} + * ({@code drill.exec.storage.upgrade.storage})</dt> + * <dd>The name of the upgrade file, normally + * {@code storage-plugins-override.conf}. Unlike th bootstrap + * file, only one upgrade file can appear in the class path. + * The upgrade file is optional.</dd> + * </dl> + */ +public class PluginBootstrapLoaderImpl implements PluginBootstrapLoader { + private static final Logger logger = LoggerFactory.getLogger(PluginBootstrapLoaderImpl.class); + + private final PluginRegistryContext context; + + /** + * The file read the first time a Drillbit starts up, and deleted + * afterwards. A poor-man's way of handling upgrades. Will be + * non-null when an upgrade is needed, null thereafter until a + * new release. + */ + private URL pluginsOverrideFileUrl; + + public PluginBootstrapLoaderImpl(PluginRegistryContext context) { + this.context = context; + } + + @Override + public StoragePlugins bootstrapPlugins() throws IOException { + StoragePlugins bootstrapPlugins = loadBootstrapPlugins(); + + // Upgrade the bootstrap plugins with any updates. Seems odd, + // but this is how Drill 1.17 works, so keeping the code. + // Uses the enabled status from the updates if different than + // the bootstrap version. + + StoragePlugins updatedPlugins = updatedPlugins(); + if (updatedPlugins != null) { + bootstrapPlugins.putAll(updatedPlugins); + } + return bootstrapPlugins; + } + + @VisibleForTesting + protected 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."); + String storageBootstrapFileName = context.config().getString(ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE); + Set<URL> storageUrls = ClassPathScanner.forResource(storageBootstrapFileName, false); + // Format plugins does not work, see note below. Review comment: Well, I am not sure we are on the same page. Let's me try to explain the intension of the written code. First we have `bootstrap-storage-plugins.json` which resides in exec module and has a list of storage plugins used as default on fresh start up. Then we have contrib module where some plugins and formats reside. We want them to be included in default plugins / formats as well during fresh start up. All works fine for the plugins, since each plugin also has `bootstrap-storage-plugins.json` and during Drill start-up they are loaded. The problem was with formats which resided in contrib module. Since when new format was added, developer had to modify `bootstrap-storage-plugins.json` in exec module to ensure new format is included. At this point `bootstrap-format-plugins.json` was added. It allowed to include formats from contrib module as bootstrap. Yes, developer has to indicate plugin name, for instance, `dfs` where format should be added but this allowed to make formats fully pluggable. This was one of the really nice improvements in the previous release. So commenting this code means you are removing previous functionality. Steps how to check that functionality works. 1. Build Drill. 2. Make sure previous build data was removed (ex: `/tmp/drill` folder). 3. Start Drill. 4. Open Web UI Storage tab. 5. Open `dfs` Storage plugin. 6. Check that it contains all formats, including those from contrib module. For example, `ltsv`. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
