drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jacq...@apache.org
Subject [1/7] drill git commit: DRILL-4000: Ensure storage plugins are not needlessly created. Add start and close capability to storage plugins.
Date Mon, 02 Nov 2015 02:56:35 GMT
Repository: drill
Updated Branches:
  refs/heads/master e4b94a784 -> ce593eb4c


DRILL-4000: Ensure storage plugins are not needlessly created. Add start and close capability
to storage plugins.

This closes #227

Add a new configuration based map so any configurations that are passed to a node can leverage
existing storage plugins.
Update FileSystemConfig to correctly implement hashcode()
Update StoragePlugin interface to extends Autocloseable and add start() method.
Update Mongo plugin to close clients when closing plugin.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/7f55051a
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/7f55051a
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/7f55051a

Branch: refs/heads/master
Commit: 7f55051a3144f0d2dfc317d426788f95606972d1
Parents: e4b94a7
Author: Jacques Nadeau <jacques@apache.org>
Authored: Sat Oct 31 15:16:32 2015 -0700
Committer: Jacques Nadeau <jacques@apache.org>
Committed: Sun Nov 1 18:53:31 2015 -0800

----------------------------------------------------------------------
 .../common/logical/StoragePluginConfig.java     |   3 +
 .../exec/store/mongo/MongoStoragePlugin.java    |   6 +
 .../drill/exec/store/AbstractStoragePlugin.java |   9 ++
 .../exec/store/NamedStoragePluginConfig.java    |   5 +
 .../apache/drill/exec/store/StoragePlugin.java  |   7 +-
 .../drill/exec/store/StoragePluginMap.java      | 120 ++++++++++++++++++
 .../drill/exec/store/StoragePluginRegistry.java | 122 +++++++++++++++----
 .../drill/exec/store/dfs/FileSystemConfig.java  |  47 ++++++-
 .../exec/store/dfs/FileSystemFormatConfig.java  |  35 ------
 .../exec/store/sys/SystemTablePluginConfig.java |   5 +
 10 files changed, 290 insertions(+), 69 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/common/src/main/java/org/apache/drill/common/logical/StoragePluginConfig.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/drill/common/logical/StoragePluginConfig.java
b/common/src/main/java/org/apache/drill/common/logical/StoragePluginConfig.java
index 308e901..a4a0f2b 100644
--- a/common/src/main/java/org/apache/drill/common/logical/StoragePluginConfig.java
+++ b/common/src/main/java/org/apache/drill/common/logical/StoragePluginConfig.java
@@ -37,4 +37,7 @@ public abstract class StoragePluginConfig{
   @Override
   public abstract boolean equals(Object o);
 
+  @Override
+  public abstract int hashCode();
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoStoragePlugin.java
----------------------------------------------------------------------
diff --git a/contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoStoragePlugin.java
b/contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoStoragePlugin.java
index 00912ea..b69ea47 100644
--- a/contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoStoragePlugin.java
+++ b/contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoStoragePlugin.java
@@ -146,4 +146,10 @@ public class MongoStoragePlugin extends AbstractStoragePlugin {
     }
     return client;
   }
+
+  @Override
+  public void close() throws Exception {
+    addressClientMap.invalidateAll();
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java
index 5e49bb0..ecebd15 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java
@@ -62,4 +62,13 @@ public abstract class AbstractStoragePlugin implements StoragePlugin{
   public AbstractGroupScan getPhysicalScan(String userName, JSONOptions selection, List<SchemaPath>
columns) throws IOException {
     throw new UnsupportedOperationException();
   }
+
+  @Override
+  public void start() throws IOException {
+  }
+
+  @Override
+  public void close() throws Exception {
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/exec/java-exec/src/main/java/org/apache/drill/exec/store/NamedStoragePluginConfig.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/NamedStoragePluginConfig.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/NamedStoragePluginConfig.java
index 23c907d..9a3b5b1 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/NamedStoragePluginConfig.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/NamedStoragePluginConfig.java
@@ -30,4 +30,9 @@ public class NamedStoragePluginConfig extends StoragePluginConfig {
     return this == o;
   }
 
+  @Override
+  public int hashCode() {
+    return name.hashCode();
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java
index 3bb5ef5..5113a18 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java
@@ -32,7 +32,7 @@ import org.apache.drill.exec.physical.base.AbstractGroupScan;
  * formats will implement methods that indicate if Drill can write or read its tables from
that format,
  * if there are optimizer rules specific for the format, getting a storage config. etc.
  */
-public interface StoragePlugin extends SchemaFactory {
+public interface StoragePlugin extends SchemaFactory, AutoCloseable {
 
   /** Indicates if Drill can read the table from this format.
   */
@@ -75,4 +75,9 @@ public interface StoragePlugin extends SchemaFactory {
   */
   public StoragePluginConfig getConfig();
 
+  /**
+   * Initialize the storage plugin. The storage plugin will not be used until this method
is called.
+   */
+  public void start() throws IOException;
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginMap.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginMap.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginMap.java
new file mode 100644
index 0000000..f12d195
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginMap.java
@@ -0,0 +1,120 @@
+/**
+ * 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.util.Collection;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentMap;
+
+import org.apache.drill.common.logical.StoragePluginConfig;
+
+import com.google.common.collect.LinkedListMultimap;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Multimaps;
+
+/**
+ * Holds maps to storage plugins. Supports name => plugin and config => plugin mappings.
+ *
+ * This is inspired by ConcurrentMap but provides a secondary key mapping that allows an
alternative lookup mechanism.
+ * The class is responsible for internally managing consistency between the two maps. This
class is threadsafe.
+ */
+class StoragePluginMap implements Iterable<Entry<String, StoragePlugin>> {
+  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(StoragePluginMap.class);
+
+  private final ConcurrentMap<String, StoragePlugin> nameMap = Maps.newConcurrentMap();
+
+  @SuppressWarnings("unchecked")
+  private final Multimap<StoragePluginConfig, StoragePlugin> configMap =
+      (Multimap<StoragePluginConfig, StoragePlugin>) (Object)
+      Multimaps.synchronizedListMultimap(LinkedListMultimap.create());
+
+  public void putAll(Map<String, StoragePlugin> mapOfPlugins) {
+    for (Entry<String, StoragePlugin> entry : mapOfPlugins.entrySet()) {
+      StoragePlugin plugin = entry.getValue();
+      nameMap.put(entry.getKey(), plugin);
+      // this possibly overwrites items in a map.
+      configMap.put(plugin.getConfig(), plugin);
+    }
+  }
+
+  public boolean replace(String name, StoragePlugin oldPlugin, StoragePlugin newPlugin) {
+    boolean ok = nameMap.replace(name, oldPlugin, newPlugin);
+    if (ok) {
+      configMap.put(newPlugin.getConfig(), newPlugin);
+      configMap.remove(oldPlugin.getConfig(), oldPlugin);
+    }
+
+    return ok;
+  }
+
+  public boolean remove(String name, StoragePlugin oldPlugin) {
+    boolean ok = nameMap.remove(name, oldPlugin);
+    if (ok) {
+      configMap.remove(oldPlugin.getConfig(), oldPlugin);
+    }
+    return ok;
+  }
+
+  public StoragePlugin putIfAbsent(String name, StoragePlugin plugin) {
+    StoragePlugin oldPlugin = nameMap.putIfAbsent(name, plugin);
+    if (oldPlugin == null) {
+      configMap.put(plugin.getConfig(), plugin);
+    }
+    return oldPlugin;
+  }
+
+  public StoragePlugin remove(String name) {
+    StoragePlugin plugin = nameMap.remove(name);
+    if (plugin != null) {
+      configMap.remove(plugin.getConfig(), plugin);
+    }
+    return plugin;
+  }
+
+  public StoragePlugin get(String name) {
+    return nameMap.get(name);
+  }
+
+  @Override
+  public Iterator<Entry<String, StoragePlugin>> iterator() {
+    return nameMap.entrySet().iterator();
+  }
+
+  public Iterable<String> names() {
+    return nameMap.keySet();
+  }
+
+  public StoragePlugin get(StoragePluginConfig config) {
+    Collection<StoragePlugin> plugins = configMap.get(config);
+    if (plugins == null || plugins.isEmpty()) {
+      return null;
+    } else {
+      // return first one since it doesn't matter which plugin we use for ephemeral purposes
(since they are all the
+      // same, they just have different names.
+      return plugins.iterator().next();
+    }
+  }
+
+  public Iterable<StoragePlugin> plugins() {
+    return nameMap.values();
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistry.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistry.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistry.java
index 4673cb5..70e82da 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistry.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistry.java
@@ -30,13 +30,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 
 import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.tools.RuleSet;
-
 import org.apache.drill.common.config.LogicalPlanPersistence;
 import org.apache.drill.common.exceptions.DrillRuntimeException;
 import org.apache.drill.common.exceptions.ExecutionSetupException;
@@ -61,8 +60,12 @@ import org.apache.drill.exec.store.sys.SystemTablePluginConfig;
 
 import com.google.common.base.Charsets;
 import com.google.common.base.Joiner;
-import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
+import com.google.common.cache.CacheBuilder;
+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.ImmutableSet;
 import com.google.common.collect.ImmutableSet.Builder;
 import com.google.common.collect.Lists;
@@ -78,13 +81,14 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
   public static final String INFORMATION_SCHEMA_PLUGIN = "INFORMATION_SCHEMA";
 
   private Map<Object, Constructor<? extends StoragePlugin>> availablePlugins
= new HashMap<Object, Constructor<? extends StoragePlugin>>();
-  private ConcurrentMap<String, StoragePlugin> plugins;
+  private final StoragePluginMap plugins = new StoragePluginMap();
 
   private DrillbitContext context;
   private final DrillSchemaFactory schemaFactory = new DrillSchemaFactory();
   private final PStore<StoragePluginConfig> pluginSystemTable;
   private final LogicalPlanPersistence lpPersistence;
   private final ScanResult classpathScan;
+  private final LoadingCache<StoragePluginConfig, StoragePlugin> ephemeralPlugins;
 
   public StoragePluginRegistry(DrillbitContext context) {
     this.context = checkNotNull(context);
@@ -101,6 +105,22 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
       logger.error("Failure while loading storage plugin registry.", e);
       throw new RuntimeException("Failure while reading and loading storage plugin configuration.",
e);
     }
+
+    ephemeralPlugins = CacheBuilder.newBuilder()
+        .expireAfterAccess(24, TimeUnit.HOURS)
+        .maximumSize(250)
+        .removalListener(new RemovalListener<StoragePluginConfig, StoragePlugin>()
{
+          @Override
+          public void onRemoval(RemovalNotification<StoragePluginConfig, StoragePlugin>
notification) {
+            closePlugin(notification.getValue());
+          }
+        })
+        .build(new CacheLoader<StoragePluginConfig, StoragePlugin>() {
+          @Override
+          public StoragePlugin load(StoragePluginConfig config) throws Exception {
+            return create(null, config);
+          }
+        });
   }
 
   public PStore<StoragePluginConfig> getStore() {
@@ -124,19 +144,20 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
             || params[1] != DrillbitContext.class
             || !StoragePluginConfig.class.isAssignableFrom(params[0])
             || params[2] != String.class) {
-          logger.info("Skipping StoragePlugin constructor {} for plugin class {} since it
doesn't implement a [constructor(StoragePluginConfig, DrillbitContext, String)]", c, plugin);
+          logger.info("Skipping StoragePlugin constructor {} for plugin class {} since it
doesn't implement a "
+              + "[constructor(StoragePluginConfig, DrillbitContext, String)]", c, plugin);
           continue;
         }
         availablePlugins.put(params[0], (Constructor<? extends StoragePlugin>) c);
         i++;
       }
       if (i == 0) {
-        logger.debug("Skipping registration of StoragePlugin {} as it doesn't have a constructor
with the parameters of (StorangePluginConfig, Config)", plugin.getCanonicalName());
+        logger.debug("Skipping registration of StoragePlugin {} as it doesn't have a constructor
with the parameters "
+            + "of (StorangePluginConfig, Config)", plugin.getCanonicalName());
       }
     }
 
     // create registered plugins defined in "storage-plugins.json"
-    this.plugins = Maps.newConcurrentMap();
     this.plugins.putAll(createPlugins());
 
   }
@@ -197,27 +218,52 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
   }
 
   public void deletePlugin(String name) {
-    plugins.remove(name);
+    StoragePlugin plugin = plugins.remove(name);
+    closePlugin(plugin);
     pluginSystemTable.delete(name);
   }
 
+  private void closePlugin(StoragePlugin plugin) {
+    if (plugin == null) {
+      return;
+    }
+
+    try {
+      plugin.close();
+    } catch (Exception e) {
+      logger.warn("Exception while shutting down storage plugin.");
+    }
+  }
+
   public StoragePlugin createOrUpdate(String name, StoragePluginConfig config, boolean persist)
throws ExecutionSetupException {
     StoragePlugin oldPlugin = plugins.get(name);
 
-    StoragePlugin newPlugin = create(name, config);
     boolean ok = true;
-    if (oldPlugin != null) {
-      if (config.isEnabled()) {
-        ok = plugins.replace(name, oldPlugin, newPlugin);
-      } else {
-        ok = plugins.remove(name, oldPlugin);
+    final StoragePlugin newPlugin = create(name, config);
+    try {
+      if (oldPlugin != null) {
+        if (config.isEnabled()) {
+          ok = plugins.replace(name, oldPlugin, newPlugin);
+          if (ok) {
+            closePlugin(oldPlugin);
+          }
+        } else {
+          ok = plugins.remove(name, oldPlugin);
+          if (ok) {
+            closePlugin(oldPlugin);
+          }
+        }
+      } else if (config.isEnabled()) {
+        ok = (null == plugins.putIfAbsent(name, newPlugin));
       }
-    } else if (config.isEnabled()) {
-      ok = (null == plugins.putIfAbsent(name, newPlugin));
-    }
 
-    if(!ok) {
-      throw new ExecutionSetupException("Two processes tried to change a plugin at the same
time.");
+      if (!ok) {
+        throw new ExecutionSetupException("Two processes tried to change a plugin at the
same time.");
+      }
+    } finally {
+      if (!ok) {
+        closePlugin(newPlugin);
+      }
     }
 
     if (persist) {
@@ -241,7 +287,9 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
       }
       return null;
     } else {
-      if (plugin == null || !plugin.getConfig().equals(config)) {
+      if (plugin == null
+          || !plugin.getConfig().equals(config)
+          || plugin.getConfig().isEnabled() != config.isEnabled()) {
         plugin = createOrUpdate(name, config, false);
       }
       return plugin;
@@ -252,8 +300,25 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
     if (config instanceof NamedStoragePluginConfig) {
       return getPlugin(((NamedStoragePluginConfig) config).name);
     } else {
-      // TODO: for now, we'll throw away transient configs. we really ought to clean these
up.
-      return create(null, config);
+      // try to lookup plugin by configuration
+      StoragePlugin plugin = plugins.get(config);
+      if (plugin != null) {
+        return plugin;
+      }
+
+      // no named plugin matches the desired configuration, let's create an
+      // ephemeral storage plugin (or get one from the cache)
+      try {
+        return ephemeralPlugins.get(config);
+      } catch (ExecutionException e) {
+        Throwable cause = e.getCause();
+        if (cause instanceof ExecutionSetupException) {
+          throw (ExecutionSetupException) cause;
+        } else {
+          // this shouldn't happen. here for completeness.
+          throw new ExecutionSetupException("Failure while trying to create ephemeral plugin.",
cause);
+        }
+      }
     }
   }
 
@@ -275,8 +340,10 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
     }
     try {
       plugin = c.newInstance(pluginConfig, context, name);
+      plugin.start();
       return plugin;
-    } catch (InstantiationException | IllegalAccessException | IllegalArgumentException |
InvocationTargetException e) {
+    } catch (InstantiationException | IllegalAccessException | IllegalArgumentException |
InvocationTargetException
+        | IOException e) {
       Throwable t = e instanceof InvocationTargetException ? ((InvocationTargetException)
e).getTargetException() : e;
       if (t instanceof ExecutionSetupException) {
         throw ((ExecutionSetupException) t);
@@ -288,13 +355,13 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
 
   @Override
   public Iterator<Entry<String, StoragePlugin>> iterator() {
-    return plugins.entrySet().iterator();
+    return plugins.iterator();
   }
 
   public RuleSet getStoragePluginRuleSet(OptimizerRulesContext optimizerRulesContext) {
     // query registered engines for optimizer rules and build the storage plugin RuleSet
     Builder<RelOptRule> setBuilder = ImmutableSet.builder();
-    for (StoragePlugin plugin : this.plugins.values()) {
+    for (StoragePlugin plugin : this.plugins.plugins()) {
       Set<? extends RelOptRule> rules = plugin.getOptimizerRules(optimizerRulesContext);
       if (rules != null && rules.size() > 0) {
         setBuilder.addAll(rules);
@@ -316,7 +383,7 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
       watch.start();
 
       try {
-        Set<String> currentPluginNames = Sets.newHashSet(plugins.keySet());
+        Set<String> currentPluginNames = Sets.newHashSet(plugins.names());
         // iterate through the plugin instances in the persistence store adding
         // any new ones and refreshing those whose configuration has changed
         for (Map.Entry<String, StoragePluginConfig> config : pluginSystemTable) {
@@ -334,7 +401,7 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
         }
 
         // finally register schemas with the refreshed plugins
-        for (StoragePlugin plugin : plugins.values()) {
+        for (StoragePlugin plugin : plugins.plugins()) {
           plugin.registerSchemas(schemaConfig, parent);
         }
       } catch (ExecutionSetupException e) {
@@ -390,4 +457,5 @@ public class StoragePluginRegistry implements Iterable<Map.Entry<String,
Storage
 
   }
 
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemConfig.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemConfig.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemConfig.java
index a64ad52..d88750f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemConfig.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemConfig.java
@@ -33,15 +33,50 @@ public class FileSystemConfig extends StoragePluginConfig {
   public Map<String, FormatPluginConfig> formats;
 
   @Override
+  public int hashCode() {
+    final int prime = 31;
+    int result = 1;
+    result = prime * result + ((connection == null) ? 0 : connection.hashCode());
+    result = prime * result + ((formats == null) ? 0 : formats.hashCode());
+    result = prime * result + ((workspaces == null) ? 0 : workspaces.hashCode());
+    return result;
+  }
+
+  @Override
   public boolean equals(Object obj) {
-    if (!(obj instanceof FileSystemConfig)) {
+    if (this == obj) {
+      return true;
+    }
+    if (obj == null) {
       return false;
     }
-    FileSystemConfig that = (FileSystemConfig) obj;
-    boolean same = ((this.connection == null && that.connection == null) || this.connection.equals(that.connection))
&&
-            ((this.workspaces == null && that.workspaces == null) || this.workspaces.equals(that.workspaces))
&&
-            ((this.formats== null && that.formats == null) || this.formats.equals(that.formats));
-    return same;
+    if (getClass() != obj.getClass()) {
+      return false;
+    }
+    FileSystemConfig other = (FileSystemConfig) obj;
+    if (connection == null) {
+      if (other.connection != null) {
+        return false;
+      }
+    } else if (!connection.equals(other.connection)) {
+      return false;
+    }
+    if (formats == null) {
+      if (other.formats != null) {
+        return false;
+      }
+    } else if (!formats.equals(other.formats)) {
+      return false;
+    }
+    if (workspaces == null) {
+      if (other.workspaces != null) {
+        return false;
+      }
+    } else if (!workspaces.equals(other.workspaces)) {
+      return false;
+    }
+    return true;
   }
 
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemFormatConfig.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemFormatConfig.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemFormatConfig.java
deleted file mode 100644
index bb363cf..0000000
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemFormatConfig.java
+++ /dev/null
@@ -1,35 +0,0 @@
-/**
- * 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.dfs;
-
-import org.apache.drill.common.logical.FormatPluginConfig;
-import org.apache.drill.common.logical.StoragePluginConfig;
-
-public class FileSystemFormatConfig<T extends FormatPluginConfig> extends StoragePluginConfig
{
-  static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FileSystemFormatConfig.class);
-
-  public T getFormatConfig(){
-    return null;
-  }
-
-  @Override
-  public boolean equals(Object o) {
-    return this == o;
-  }
-
-}

http://git-wip-us.apache.org/repos/asf/drill/blob/7f55051a/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTablePluginConfig.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTablePluginConfig.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTablePluginConfig.java
index b3348a2..509cfe7 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTablePluginConfig.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTablePluginConfig.java
@@ -37,4 +37,9 @@ public class SystemTablePluginConfig extends StoragePluginConfig {
     return this == o;
   }
 
+  @Override
+  public int hashCode() {
+    return 1;
+  }
+
 }


Mime
View raw message