drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From prog...@apache.org
Subject drill git commit: DRILL-5809 made the storage format of system options backwards compatible, and avoided storing unnecessary option info.
Date Tue, 26 Sep 2017 06:20:00 GMT
Repository: drill
Updated Branches:
  refs/heads/master 44d5cc8eb -> 8a8bf63f7


DRILL-5809 made the storage format of system options backwards compatible, and avoided storing unnecessary option info.

 - Fixed forward compatability issue
 - Added error message for debugging
 - Fix flakey test
 - Cleaned up bad logging
 - Applied comments

closes #957


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

Branch: refs/heads/master
Commit: 8a8bf63f7e9f804e761c69f8e94f34417f83c7f7
Parents: 44d5cc8
Author: Timothy Farkas <timothyfarkas@apache.org>
Authored: Thu Sep 21 22:40:46 2017 -0700
Committer: Paul Rogers <progers@maprtech.com>
Committed: Mon Sep 25 22:08:45 2017 -0700

----------------------------------------------------------------------
 .../apache/drill/testutils/DirTestWatcher.java  |  61 ++++
 .../exec/server/options/BaseOptionManager.java  |   2 +-
 .../exec/server/options/OptionMetaData.java     |   2 +-
 .../drill/exec/server/options/OptionValue.java  |  60 +++-
 .../server/options/PersistedOptionValue.java    | 316 +++++++++++++++++++
 .../server/options/SystemOptionManager.java     |  34 +-
 .../java/org/apache/drill/BaseTestQuery.java    |  14 +-
 .../TestInboundImpersonationPrivileges.java     |   2 +-
 .../exec/server/options/OptionValueTest.java    |  72 +++++
 .../options/PersistedOptionValueTest.java       | 226 +++++++++++++
 .../exec/store/sys/TestPStoreProviders.java     | 121 ++++++-
 .../testing/TestCountDownLatchInjection.java    |   7 +-
 .../drill/exec/util/MiniZooKeeperCluster.java   |   4 +-
 .../test/resources/options/old_booleanopt.json  |   6 +
 .../test/resources/options/old_doubleopt.json   |   6 +
 .../src/test/resources/options/old_longopt.json |   6 +
 .../test/resources/options/old_stringopt.json   |   6 +
 ...rowgroup.filter.pushdown.threshold.sys.drill |   6 +
 .../planner.width.max_per_query.sys.drill       |   6 +
 pom.xml                                         |   1 +
 20 files changed, 910 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/common/src/test/java/org/apache/drill/testutils/DirTestWatcher.java
----------------------------------------------------------------------
diff --git a/common/src/test/java/org/apache/drill/testutils/DirTestWatcher.java b/common/src/test/java/org/apache/drill/testutils/DirTestWatcher.java
new file mode 100644
index 0000000..a5a8806
--- /dev/null
+++ b/common/src/test/java/org/apache/drill/testutils/DirTestWatcher.java
@@ -0,0 +1,61 @@
+/*
+ * 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.testutils;
+
+import org.apache.commons.io.FileUtils;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+
+import java.io.File;
+
+/**
+ * This JUnit {@link TestWatcher} creates a unique directory for each JUnit test in the project's
+ * target folder at the start of each test. This directory can be used as a temporary directory to store
+ * files for the test. The directory and its contents are deleted at the end of the test.
+ */
+public class DirTestWatcher extends TestWatcher {
+  private String dirPath;
+  private File dir;
+
+  @Override
+  protected void starting(Description description) {
+    dirPath = String.format("%s/target/%s/%s", new File(".").getAbsolutePath(),
+      description.getClassName(), description.getMethodName());
+    dir = new File(dirPath);
+    dir.mkdirs();
+  }
+
+  @Override
+  protected void finished(Description description) {
+    FileUtils.deleteQuietly(dir);
+  }
+
+  @Override
+  protected void failed(Throwable e, Description description) {
+    FileUtils.deleteQuietly(dir);
+  }
+
+  public String getDirPath() {
+    return dirPath;
+  }
+
+  public File getDir() {
+    return dir;
+  }
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java
index 26f9108..5a0fa2d 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java
@@ -62,7 +62,7 @@ public abstract class BaseOptionManager extends BaseOptionSet implements OptionM
     final OptionDefinition definition = getOptionDefinition(name);
     final OptionValidator validator = definition.getValidator();
     final OptionMetaData metaData = definition.getMetaData();
-    final OptionValue.AccessibleScopes type = definition.getMetaData().getType();
+    final OptionValue.AccessibleScopes type = definition.getMetaData().getAccessibleScopes();
     final OptionValue.OptionScope scope = getScope();
     checkOptionPermissions(name, type, scope);
     final OptionValue optionValue = OptionValue.create(type, name, value, scope);

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionMetaData.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionMetaData.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionMetaData.java
index f73c348..6d2762f 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionMetaData.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionMetaData.java
@@ -34,7 +34,7 @@ public class OptionMetaData {
     this.internal = internal;
   }
 
-  public OptionValue.AccessibleScopes getType() {
+  public OptionValue.AccessibleScopes getAccessibleScopes() {
     return type;
   }
 

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java
index 0a49f5d..e14a846 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionValue.java
@@ -44,6 +44,14 @@ import java.util.EnumSet;
  */
 @JsonInclude(Include.NON_NULL)
 public class OptionValue implements Comparable<OptionValue> {
+  public static final String JSON_KIND = "kind";
+  public static final String JSON_ACCESSIBLE_SCOPES = "accessibleScopes";
+  public static final String JSON_NAME = "name";
+  public static final String JSON_NUM_VAL = "num_val";
+  public static final String JSON_STRING_VAL = "string_val";
+  public static final String JSON_BOOL_VAL = "bool_val";
+  public static final String JSON_FLOAT_VAL = "float_val";
+  public static final String JSON_SCOPE = "scope";
 
   /**
    * Defines where an option can be configured.
@@ -88,20 +96,36 @@ public class OptionValue implements Comparable<OptionValue> {
   public final Double float_val;
   public final OptionScope scope;
 
-  public static OptionValue create(AccessibleScopes type, String name, long val, OptionScope scope) {
-    return new OptionValue(Kind.LONG, type, name, val, null, null, null, scope);
+  public static OptionValue create(AccessibleScopes accessibleScopes, String name, long val, OptionScope scope) {
+    return new OptionValue(Kind.LONG, accessibleScopes, name, val, null, null, null, scope);
   }
 
-  public static OptionValue create(AccessibleScopes type, String name, boolean bool, OptionScope scope) {
-    return new OptionValue(Kind.BOOLEAN, type, name, null, null, bool, null, scope);
+  public static OptionValue create(AccessibleScopes accessibleScopes, String name, boolean bool, OptionScope scope) {
+    return new OptionValue(Kind.BOOLEAN, accessibleScopes, name, null, null, bool, null, scope);
   }
 
-  public static OptionValue create(AccessibleScopes type, String name, String val, OptionScope scope) {
-    return new OptionValue(Kind.STRING, type, name, null, val, null, null, scope);
+  public static OptionValue create(AccessibleScopes accessibleScopes, String name, String val, OptionScope scope) {
+    return new OptionValue(Kind.STRING, accessibleScopes, name, null, val, null, null, scope);
   }
 
-  public static OptionValue create(AccessibleScopes type, String name, double val, OptionScope scope) {
-    return new OptionValue(Kind.DOUBLE, type, name, null, null, null, val, scope);
+  public static OptionValue create(AccessibleScopes accessibleScopes, String name, double val, OptionScope scope) {
+    return new OptionValue(Kind.DOUBLE, accessibleScopes, name, null, null, null, val, scope);
+  }
+
+  public static OptionValue create(Kind kind, AccessibleScopes accessibleScopes,
+                                   String name, String val, OptionScope scope) {
+    switch (kind) {
+      case BOOLEAN:
+        return create(accessibleScopes, name, Boolean.valueOf(val), scope);
+      case STRING:
+        return create(accessibleScopes, name, val, scope);
+      case DOUBLE:
+        return create(accessibleScopes, name, Double.parseDouble(val), scope);
+      case LONG:
+        return create(accessibleScopes, name, Long.parseLong(val), scope);
+      default:
+        throw new IllegalArgumentException(String.format("Unsupported kind %s", kind));
+    }
   }
 
   public static OptionValue create(AccessibleScopes type, String name, Object val, OptionScope scope) {
@@ -119,14 +143,14 @@ public class OptionValue implements Comparable<OptionValue> {
   }
 
   @JsonCreator
-  private OptionValue(@JsonProperty("kind") Kind kind,
-                      @JsonProperty("accessibleScopes") AccessibleScopes accessibleScopes,
-                      @JsonProperty("name") String name,
-                      @JsonProperty("num_val") Long num_val,
-                      @JsonProperty("string_val") String string_val,
-                      @JsonProperty("bool_val") Boolean bool_val,
-                      @JsonProperty("float_val") Double float_val,
-                      @JsonProperty("scope") OptionScope scope) {
+  private OptionValue(@JsonProperty(JSON_KIND) Kind kind,
+                      @JsonProperty(JSON_ACCESSIBLE_SCOPES) AccessibleScopes accessibleScopes,
+                      @JsonProperty(JSON_NAME) String name,
+                      @JsonProperty(JSON_NUM_VAL) Long num_val,
+                      @JsonProperty(JSON_STRING_VAL) String string_val,
+                      @JsonProperty(JSON_BOOL_VAL) Boolean bool_val,
+                      @JsonProperty(JSON_FLOAT_VAL) Double float_val,
+                      @JsonProperty(JSON_SCOPE) OptionScope scope) {
     Preconditions.checkArgument(num_val != null || string_val != null || bool_val != null || float_val != null);
     this.kind = kind;
     this.accessibleScopes = accessibleScopes;
@@ -158,6 +182,10 @@ public class OptionValue implements Comparable<OptionValue> {
     }
   }
 
+  public PersistedOptionValue toPersisted() {
+    return new PersistedOptionValue(kind, name, num_val, string_val, bool_val, float_val);
+  }
+
   @Override
   public int hashCode() {
     final int prime = 31;

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/PersistedOptionValue.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/PersistedOptionValue.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/PersistedOptionValue.java
new file mode 100644
index 0000000..685799c
--- /dev/null
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/PersistedOptionValue.java
@@ -0,0 +1,316 @@
+/*
+ * 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.server.options;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.core.ObjectCodec;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JavaType;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
+import com.google.common.base.Preconditions;
+
+import java.io.IOException;
+
+/**
+ * <p>
+ * This represents a persisted {@link OptionValue}. Decoupling the {@link OptionValue} from what
+ * is persisted will prevent us from accidentally breaking backward compatibility in the future
+ * when the {@link OptionValue} changes. Additionally when we do change the format of stored options we
+ * will not have to change much code since this is already designed with backward compatibility in mind.
+ * This class is also forward compatible with the Drill Option storage format in Drill 1.11 and earlier.
+ * </p>
+ *
+ * <p>
+ * <b>Contract:</b>
+ * Only {@link PersistedOptionValue}s created from an {@link OptionValue} should be persisted.
+ * And {@link OptionValue}s should only be created from {@link PersistedOptionValue}s that are
+ * retrieved from a store.
+ * </p>
+ */
+
+// Custom Deserializer required for backward compatibility DRILL-5809
+@JsonInclude(JsonInclude.Include.NON_NULL)
+@JsonDeserialize(using = PersistedOptionValue.Deserializer.class)
+public class PersistedOptionValue {
+  /**
+   * This is present for forward compatability with Drill 1.11 and earlier
+   */
+  public static final String SYSTEM_TYPE = "SYSTEM";
+  /**
+   * This constant cannot be changed for backward and forward compatibility reasons.
+   */
+  public static final String JSON_TYPE = "type";
+  /**
+   * This constant cannot be changed for backward and forward compatibility reasons.
+   */
+  public static final String JSON_KIND = "kind";
+  /**
+   * This constant cannot be changed for backward and forward compatibility reasons.
+   */
+  public static final String JSON_NAME = "name";
+  /**
+   * This constant cannot be changed for backward and forward compatibility reasons.
+   */
+  public static final String JSON_NUM_VAL = "num_val";
+  /**
+   * This constant cannot be changed for backward and forward compatibility reasons.
+   */
+  public static final String JSON_STRING_VAL = "string_val";
+  /**
+   * This constant cannot be changed for backward and forward compatibility reasons.
+   */
+  public static final String JSON_BOOL_VAL = "bool_val";
+  /**
+   * This constant cannot be changed for backward and forward compatibility reasons.
+   */
+  public static final String JSON_FLOAT_VAL = "float_val";
+
+  private String value;
+  private OptionValue.Kind kind;
+  private String name;
+  private Long num_val;
+  private String string_val;
+  private Boolean bool_val;
+  private Double float_val;
+
+  public PersistedOptionValue(String value) {
+    this.value = Preconditions.checkNotNull(value);
+  }
+
+  public PersistedOptionValue(OptionValue.Kind kind, String name,
+                              Long num_val, String string_val,
+                              Boolean bool_val, Double float_val) {
+    this.kind = kind;
+    this.name = name;
+    this.num_val = num_val;
+    this.string_val = string_val;
+    this.bool_val = bool_val;
+    this.float_val = float_val;
+
+    switch (kind) {
+      case BOOLEAN:
+        Preconditions.checkNotNull(bool_val);
+        value = bool_val.toString();
+        break;
+      case STRING:
+        Preconditions.checkNotNull(string_val);
+        value = string_val;
+        break;
+      case DOUBLE:
+        Preconditions.checkNotNull(float_val);
+        value = float_val.toString();
+        break;
+      case LONG:
+        Preconditions.checkNotNull(num_val);
+        value = num_val.toString();
+        break;
+      default:
+        throw new UnsupportedOperationException(String.format("Unsupported type %s", kind));
+    }
+  }
+
+  /**
+   * This is ignored for forward compatibility.
+   */
+  @JsonIgnore
+  public String getValue() {
+    return value;
+  }
+
+  /**
+   * This is present for forward compatibility.
+   */
+  @JsonProperty(JSON_TYPE)
+  public String getType() {
+    return SYSTEM_TYPE;
+  }
+
+  /**
+   * This is present for forward compatibility.
+   */
+  @JsonProperty(JSON_KIND)
+  public OptionValue.Kind getKind() {
+    return kind;
+  }
+
+  /**
+   * This is present for forward compatibility.
+   */
+  @JsonProperty(JSON_NAME)
+  public String getName() {
+    return name;
+  }
+
+  /**
+   * This is present for forward compatibility.
+   */
+  @JsonProperty(JSON_NUM_VAL)
+  public Long getNumVal() {
+    return num_val;
+  }
+
+  /**
+   * This is present for forward compatibility.
+   */
+  @JsonProperty(JSON_STRING_VAL)
+  public String getStringVal() {
+    return string_val;
+  }
+
+  /**
+   * This is present for forward compatibility.
+   */
+  @JsonProperty(JSON_BOOL_VAL)
+  public Boolean getBoolVal() {
+    return bool_val;
+  }
+
+  /**
+   * This is present for forward compatibility.
+   */
+  @JsonProperty(JSON_FLOAT_VAL)
+  public Double getFloatVal() {
+    return float_val;
+  }
+
+  public OptionValue toOptionValue(final OptionDefinition optionDefinition, final OptionValue.OptionScope optionScope) {
+    Preconditions.checkNotNull(value, "The value must be defined in order for this to be converted to an " +
+    "option value");
+    final OptionValidator validator = optionDefinition.getValidator();
+    final OptionValue.Kind kind = validator.getKind();
+    final String name = validator.getOptionName();
+    final OptionValue.AccessibleScopes accessibleScopes = optionDefinition.getMetaData().getAccessibleScopes();
+    return OptionValue.create(kind, accessibleScopes, name, value, optionScope);
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+
+    PersistedOptionValue that = (PersistedOptionValue) o;
+
+    if (value != null ? !value.equals(that.value) : that.value != null) {
+      return false;
+    }
+
+    if (kind != that.kind) {
+      return false;
+    }
+
+    if (name != null ? !name.equals(that.name) : that.name != null) {
+      return false;
+    }
+
+    if (num_val != null ? !num_val.equals(that.num_val) : that.num_val != null) {
+      return false;
+    }
+
+    if (string_val != null ? !string_val.equals(that.string_val) : that.string_val != null) {
+      return false;
+    }
+
+    if (bool_val != null ? !bool_val.equals(that.bool_val) : that.bool_val != null) {
+      return false;
+    }
+
+    return float_val != null ? float_val.equals(that.float_val) : that.float_val == null;
+  }
+
+  @Override
+  public int hashCode() {
+    int result = value != null ? value.hashCode() : 0;
+    result = 31 * result + (kind != null ? kind.hashCode() : 0);
+    result = 31 * result + (name != null ? name.hashCode() : 0);
+    result = 31 * result + (num_val != null ? num_val.hashCode() : 0);
+    result = 31 * result + (string_val != null ? string_val.hashCode() : 0);
+    result = 31 * result + (bool_val != null ? bool_val.hashCode() : 0);
+    result = 31 * result + (float_val != null ? float_val.hashCode() : 0);
+    return result;
+  }
+
+  @Override
+  public String toString() {
+    return "PersistedOptionValue{" + "value='" + value + '\'' + ", kind=" + kind + ", name='" + name +
+      '\'' + ", num_val=" + num_val + ", string_val='" + string_val + '\'' + ", bool_val=" + bool_val +
+      ", float_val=" + float_val + '}';
+  }
+
+  /**
+   * This deserializer only fetches the relevant information we care about from a store, which is the
+   * value of an option. This deserializer is essentially future proof since it only requires a value
+   * to be stored for an option.
+   */
+  public static class Deserializer extends StdDeserializer<PersistedOptionValue> {
+    private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(Deserializer.class);
+
+    private Deserializer() {
+      super(PersistedOptionValue.class);
+    }
+
+    protected Deserializer(JavaType valueType) {
+      super(valueType);
+    }
+
+    protected Deserializer(StdDeserializer<?> src) {
+      super(src);
+    }
+
+    @Override
+    public PersistedOptionValue deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JsonProcessingException {
+      ObjectCodec oc = p.getCodec();
+      JsonNode node = oc.readTree(p);
+      String value = null;
+
+      if (node.has(OptionValue.JSON_NUM_VAL)) {
+        value = node.get(OptionValue.JSON_NUM_VAL).asText();
+      }
+
+      if (node.has(OptionValue.JSON_STRING_VAL)) {
+        value = node.get(OptionValue.JSON_STRING_VAL).asText();
+      }
+
+      if (node.has(OptionValue.JSON_BOOL_VAL)) {
+        value = node.get(OptionValue.JSON_BOOL_VAL).asText();
+      }
+
+      if (node.has(OptionValue.JSON_FLOAT_VAL)) {
+        value = node.get(OptionValue.JSON_FLOAT_VAL).asText();
+      }
+
+      if (value == null) {
+        logger.error("Invalid json stored {}.", new ObjectMapper().writeValueAsString(node));
+      }
+
+      return new PersistedOptionValue(value);
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
index d1d56a2..b0863ee 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java
@@ -17,8 +17,6 @@
  */
 package org.apache.drill.exec.server.options;
 
-import static com.google.common.base.Preconditions.checkArgument;
-
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -218,7 +216,7 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
     return map;
   }
 
-  private final PersistentStoreConfig<OptionValue> config;
+  private final PersistentStoreConfig<PersistedOptionValue> config;
 
   private final PersistentStoreProvider provider;
 
@@ -227,7 +225,7 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
    * Persistent store for options that have been changed from default.
    * NOTE: CRUD operations must use lowercase keys.
    */
-  private PersistentStore<OptionValue> options;
+  private PersistentStore<PersistedOptionValue> options;
   private CaseInsensitiveMap<OptionDefinition> definitions;
   private CaseInsensitiveMap<OptionValue> defaults;
 
@@ -239,7 +237,7 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
   public SystemOptionManager(final LogicalPlanPersistence lpPersistence, final PersistentStoreProvider provider,
                              final DrillConfig bootConfig, final CaseInsensitiveMap<OptionDefinition> definitions) {
     this.provider = provider;
-    this.config = PersistentStoreConfig.newJacksonBuilder(lpPersistence.getMapper(), OptionValue.class)
+    this.config = PersistentStoreConfig.newJacksonBuilder(lpPersistence.getMapper(), PersistedOptionValue.class)
           .name("sys.options")
           .build();
     this.bootConfig = bootConfig;
@@ -256,7 +254,7 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
   public SystemOptionManager init() throws Exception {
     options = provider.getOrCreateStore(config);
     // if necessary, deprecate and replace options from persistent store
-    for (final Entry<String, OptionValue> option : Lists.newArrayList(options.getAll())) {
+    for (final Entry<String, PersistedOptionValue> option : Lists.newArrayList(options.getAll())) {
       final String name = option.getKey();
       final OptionDefinition definition = definitions.get(name);
       if (definition == null) {
@@ -269,7 +267,7 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
         if (!name.equals(canonicalName)) {
           // for backwards compatibility <= 1.1, rename to lower case.
           logger.warn("Changing option name to lower case `{}`", name);
-          final OptionValue value = option.getValue();
+          final PersistedOptionValue value = option.getValue();
           options.delete(name);
           options.put(canonicalName, value);
         }
@@ -287,8 +285,13 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
       buildList.put(entry.getKey(), entry.getValue());
     }
     // override if changed
-    for (final Map.Entry<String, OptionValue> entry : Lists.newArrayList(options.getAll())) {
-      buildList.put(entry.getKey(), entry.getValue());
+    for (final Map.Entry<String, PersistedOptionValue> entry : Lists.newArrayList(options.getAll())) {
+      final String name = entry.getKey();
+      final OptionDefinition optionDefinition = getOptionDefinition(name);
+      final PersistedOptionValue persistedOptionValue = entry.getValue();
+      final OptionValue optionValue = persistedOptionValue
+        .toOptionValue(optionDefinition, OptionValue.OptionScope.SYSTEM);
+      buildList.put(name, optionValue);
     }
     return buildList.values().iterator();
   }
@@ -296,10 +299,11 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
   @Override
   public OptionValue getOption(final String name) {
     // check local space (persistent store)
-    final OptionValue value = options.get(name.toLowerCase());
+    final PersistedOptionValue persistedValue = options.get(name.toLowerCase());
 
-    if (value != null) {
-      return value;
+    if (persistedValue != null) {
+      final OptionDefinition optionDefinition = getOptionDefinition(name);
+      return persistedValue.toOptionValue(optionDefinition, OptionValue.OptionScope.SYSTEM);
     }
 
     // otherwise, return default set in the validator.
@@ -323,7 +327,7 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
       return; // if the option is not overridden, ignore setting option to default
     }
 
-    options.put(name, value);
+    options.put(name, value.toPersisted());
   }
 
   @Override
@@ -340,7 +344,7 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
   @Override
   public void deleteAllLocalOptions() {
     final Set<String> names = Sets.newHashSet();
-    for (final Map.Entry<String, OptionValue> entry : Lists.newArrayList(options.getAll())) {
+    for (final Map.Entry<String, PersistedOptionValue> entry : Lists.newArrayList(options.getAll())) {
       names.add(entry.getKey());
     }
     for (final String name : names) {
@@ -355,7 +359,7 @@ public class SystemOptionManager extends BaseOptionManager implements AutoClosea
     for (final Map.Entry<String, OptionDefinition> entry : definitions.entrySet()) {
       final OptionDefinition definition = entry.getValue();
       final OptionMetaData metaData = definition.getMetaData();
-      final OptionValue.AccessibleScopes type = metaData.getType();
+      final OptionValue.AccessibleScopes type = metaData.getAccessibleScopes();
       final OptionValidator validator = definition.getValidator();
       final String name = validator.getOptionName();
       final String configName = validator.getConfigProperty();

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
index e30c5e9..d40233d 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
@@ -92,6 +92,8 @@ public class BaseTestQuery extends ExecTest {
     {
       put(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, "false");
       put(ExecConstants.HTTP_ENABLE, "false");
+      // Increasing retry attempts for testing
+      put(ExecConstants.UDF_RETRY_ATTEMPTS, "10");
     }
   };
 
@@ -198,6 +200,10 @@ public class BaseTestQuery extends ExecTest {
   }
 
   private static void openClient(Properties properties) throws Exception {
+    if (properties == null) {
+      properties = new Properties();
+    }
+
     allocator = RootAllocatorFactory.newRoot(config);
     serviceSet = RemoteServiceSet.getLocalServiceSet();
 
@@ -213,7 +219,13 @@ public class BaseTestQuery extends ExecTest {
       TestUtilities.makeDfsTmpSchemaImmutable(pluginRegistry);
     }
 
-    client = QueryTestUtil.createClient(config,  serviceSet, MAX_WIDTH_PER_NODE, properties);
+    if (!properties.containsKey(DrillProperties.DRILLBIT_CONNECTION)) {
+      properties = new Properties(properties);
+      properties.setProperty(DrillProperties.DRILLBIT_CONNECTION,
+        String.format("localhost:%s", bits[0].getUserPort()));
+    }
+
+    client = QueryTestUtil.createClient(config, serviceSet, MAX_WIDTH_PER_NODE, properties);
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestInboundImpersonationPrivileges.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestInboundImpersonationPrivileges.java b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestInboundImpersonationPrivileges.java
index f452669..1c656e0 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestInboundImpersonationPrivileges.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestInboundImpersonationPrivileges.java
@@ -50,7 +50,7 @@ public class TestInboundImpersonationPrivileges extends BaseTestImpersonation {
   private static boolean checkPrivileges(final String proxyName, final String targetName) {
     OptionDefinition optionDefinition = SystemOptionManager.createDefaultOptionDefinitions().get(ExecConstants.IMPERSONATION_POLICIES_KEY);
     ExecConstants.IMPERSONATION_POLICY_VALIDATOR.validate(
-        OptionValue.create(optionDefinition.getMetaData().getType(),
+        OptionValue.create(optionDefinition.getMetaData().getAccessibleScopes(),
             ExecConstants.IMPERSONATION_POLICIES_KEY,
             IMPERSONATION_POLICIES,OptionValue.OptionScope.SYSTEM), optionDefinition.getMetaData(),null);
     try {

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/java/org/apache/drill/exec/server/options/OptionValueTest.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/server/options/OptionValueTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/server/options/OptionValueTest.java
new file mode 100644
index 0000000..5f674e8
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/server/options/OptionValueTest.java
@@ -0,0 +1,72 @@
+/*
+ * 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.server.options;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class OptionValueTest {
+  @Test
+  public void createBooleanKindTest() {
+    final OptionValue createdValue = OptionValue.create(
+      OptionValue.Kind.BOOLEAN, OptionValue.AccessibleScopes.ALL,
+      "myOption", "true", OptionValue.OptionScope.SYSTEM);
+
+    final OptionValue expectedValue = OptionValue.create(
+      OptionValue.AccessibleScopes.ALL, "myOption", true, OptionValue.OptionScope.SYSTEM);
+
+    Assert.assertEquals(expectedValue, createdValue);
+  }
+
+  @Test
+  public void createDoubleKindTest() {
+    final OptionValue createdValue = OptionValue.create(
+      OptionValue.Kind.DOUBLE, OptionValue.AccessibleScopes.ALL,
+      "myOption", "1.5", OptionValue.OptionScope.SYSTEM);
+
+    final OptionValue expectedValue = OptionValue.create(
+      OptionValue.AccessibleScopes.ALL, "myOption", 1.5, OptionValue.OptionScope.SYSTEM);
+
+    Assert.assertEquals(expectedValue, createdValue);
+  }
+
+  @Test
+  public void createLongKindTest() {
+    final OptionValue createdValue = OptionValue.create(
+      OptionValue.Kind.LONG, OptionValue.AccessibleScopes.ALL,
+      "myOption", "3000", OptionValue.OptionScope.SYSTEM);
+
+    final OptionValue expectedValue = OptionValue.create(
+      OptionValue.AccessibleScopes.ALL, "myOption", 3000, OptionValue.OptionScope.SYSTEM);
+
+    Assert.assertEquals(expectedValue, createdValue);
+  }
+
+  @Test
+  public void createStringKindTest() {
+    final OptionValue createdValue = OptionValue.create(
+      OptionValue.Kind.STRING, OptionValue.AccessibleScopes.ALL,
+      "myOption", "wabalubawubdub", OptionValue.OptionScope.SYSTEM);
+
+    final OptionValue expectedValue = OptionValue.create(
+      OptionValue.AccessibleScopes.ALL, "myOption", "wabalubawubdub", OptionValue.OptionScope.SYSTEM);
+
+    Assert.assertEquals(expectedValue, createdValue);
+  }
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/java/org/apache/drill/exec/server/options/PersistedOptionValueTest.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/server/options/PersistedOptionValueTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/server/options/PersistedOptionValueTest.java
new file mode 100644
index 0000000..182442b
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/server/options/PersistedOptionValueTest.java
@@ -0,0 +1,226 @@
+/*
+ * 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.server.options;
+
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.drill.common.util.FileUtils;
+import org.apache.drill.exec.serialization.JacksonSerializer;
+import org.apache.drill.exec.store.sys.PersistentStoreConfig;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+
+public class PersistedOptionValueTest {
+  /**
+   * DRILL-5809
+   * Note: If this test breaks you are probably breaking backward and forward compatibility. Verify with the community
+   * that breaking compatibility is acceptable and planned for.
+   * @throws Exception
+   */
+  @Test
+  public void oldDeserializeTest() throws IOException {
+    testHelper("/options/old_booleanopt.json",
+      "/options/old_doubleopt.json",
+      "/options/old_longopt.json",
+      "/options/old_stringopt.json");
+  }
+
+  private void testHelper(String booleanOptionFile, String doubleOptionFile,
+                          String longOptionFile, String stringOptionFile) throws IOException {
+    JacksonSerializer serializer = new JacksonSerializer<>(new ObjectMapper(), PersistedOptionValue.class);
+    String booleanOptionJson = FileUtils.getResourceAsString(booleanOptionFile);
+    String doubleOptionJson = FileUtils.getResourceAsString(doubleOptionFile);
+    String longOptionJson = FileUtils.getResourceAsString(longOptionFile);
+    String stringOptionJson = FileUtils.getResourceAsString(stringOptionFile);
+
+    PersistedOptionValue booleanValue = (PersistedOptionValue) serializer.deserialize(booleanOptionJson.getBytes());
+    PersistedOptionValue doubleValue = (PersistedOptionValue) serializer.deserialize(doubleOptionJson.getBytes());
+    PersistedOptionValue longValue = (PersistedOptionValue) serializer.deserialize(longOptionJson.getBytes());
+    PersistedOptionValue stringValue = (PersistedOptionValue) serializer.deserialize(stringOptionJson.getBytes());
+
+    PersistedOptionValue expectedBooleanValue = new PersistedOptionValue("true");
+    PersistedOptionValue expectedDoubleValue = new PersistedOptionValue("1.5");
+    PersistedOptionValue expectedLongValue = new PersistedOptionValue("5000");
+    PersistedOptionValue expectedStringValue = new PersistedOptionValue("wabalubadubdub");
+
+    Assert.assertEquals(expectedBooleanValue, booleanValue);
+    Assert.assertEquals(expectedDoubleValue, doubleValue);
+    Assert.assertEquals(expectedLongValue, longValue);
+    Assert.assertEquals(expectedStringValue, stringValue);
+  }
+
+  @Test
+  public void valueAssignment() {
+    final String name = "myOption";
+    final String stringContent = "val1";
+    PersistedOptionValue stringValue =
+      new PersistedOptionValue(OptionValue.Kind.STRING, name, null, stringContent, null, null);
+    PersistedOptionValue numValue =
+      new PersistedOptionValue(OptionValue.Kind.LONG, name, 100L, null, null, null);
+    PersistedOptionValue boolValue =
+      new PersistedOptionValue(OptionValue.Kind.BOOLEAN, name, null, null, true, null);
+    PersistedOptionValue floatValue =
+      new PersistedOptionValue(OptionValue.Kind.DOUBLE, name, null, null, null, 55.5);
+
+    Assert.assertEquals(stringContent, stringValue.getValue());
+    Assert.assertEquals("100", numValue.getValue());
+    Assert.assertEquals("true", boolValue.getValue());
+    Assert.assertEquals("55.5", floatValue.getValue());
+  }
+
+  /**
+   * DRILL-5809 Test forward compatibility with Drill 1.11 and earlier.
+   * Note: If this test breaks you are probably breaking forward compatibility. Verify with the community
+   * that breaking compatibility is acceptable and planned for.
+   * @throws Exception
+   */
+  @Test
+  public void testForwardCompatibility() throws IOException {
+    final String name = "myOption";
+
+    JacksonSerializer realSerializer = new JacksonSerializer<>(new ObjectMapper(), PersistedOptionValue.class);
+    JacksonSerializer mockSerializer = new JacksonSerializer<>(new ObjectMapper(), MockPersistedOptionValue.class);
+
+    final String stringContent = "val1";
+    PersistedOptionValue stringValue =
+      new PersistedOptionValue(OptionValue.Kind.STRING, name, null, stringContent, null, null);
+    PersistedOptionValue numValue =
+      new PersistedOptionValue(OptionValue.Kind.LONG, name, 100L, null, null, null);
+    PersistedOptionValue boolValue =
+      new PersistedOptionValue(OptionValue.Kind.BOOLEAN, name, null, null, true, null);
+    PersistedOptionValue floatValue =
+      new PersistedOptionValue(OptionValue.Kind.DOUBLE, name, null, null, null, 55.5);
+
+    byte[] stringValueBytes = realSerializer.serialize(stringValue);
+    byte[] numValueBytes = realSerializer.serialize(numValue);
+    byte[] boolValueBytes = realSerializer.serialize(boolValue);
+    byte[] floatValueBytes = realSerializer.serialize(floatValue);
+
+    MockPersistedOptionValue mockStringValue = (MockPersistedOptionValue) mockSerializer.deserialize(stringValueBytes);
+    MockPersistedOptionValue mockNumValue = (MockPersistedOptionValue) mockSerializer.deserialize(numValueBytes);
+    MockPersistedOptionValue mockBoolValue = (MockPersistedOptionValue) mockSerializer.deserialize(boolValueBytes);
+    MockPersistedOptionValue mockFloatValue = (MockPersistedOptionValue) mockSerializer.deserialize(floatValueBytes);
+
+    MockPersistedOptionValue expectedStringValue =
+      new MockPersistedOptionValue(PersistedOptionValue.SYSTEM_TYPE, OptionValue.Kind.STRING, name,
+        null, stringContent, null, null);
+    MockPersistedOptionValue expectedNumValue =
+      new MockPersistedOptionValue(PersistedOptionValue.SYSTEM_TYPE, OptionValue.Kind.LONG, name,
+        100L, null, null, null);
+    MockPersistedOptionValue expectedBoolValue =
+      new MockPersistedOptionValue(PersistedOptionValue.SYSTEM_TYPE, OptionValue.Kind.BOOLEAN, name,
+        null, null, true, null);
+    MockPersistedOptionValue expectedFloatValue =
+      new MockPersistedOptionValue(PersistedOptionValue.SYSTEM_TYPE, OptionValue.Kind.DOUBLE, name,
+        null, null, null, 55.5);
+
+    Assert.assertEquals(expectedStringValue, mockStringValue);
+    Assert.assertEquals(expectedNumValue, mockNumValue);
+    Assert.assertEquals(expectedBoolValue, mockBoolValue);
+    Assert.assertEquals(expectedFloatValue, mockFloatValue);
+  }
+
+  @JsonInclude(JsonInclude.Include.NON_NULL)
+  public static class MockPersistedOptionValue {
+    public final String type;
+    public final OptionValue.Kind kind;
+    public final String name;
+    public final Long num_val;
+    public final String string_val;
+    public final Boolean bool_val;
+    public final Double float_val;
+
+    public MockPersistedOptionValue(@JsonProperty(PersistedOptionValue.JSON_TYPE) String type,
+                                    @JsonProperty(PersistedOptionValue.JSON_KIND) OptionValue.Kind kind,
+                                    @JsonProperty(PersistedOptionValue.JSON_NAME) String name,
+                                    @JsonProperty(PersistedOptionValue.JSON_NUM_VAL) Long num_val,
+                                    @JsonProperty(PersistedOptionValue.JSON_STRING_VAL) String string_val,
+                                    @JsonProperty(PersistedOptionValue.JSON_BOOL_VAL) Boolean bool_val,
+                                    @JsonProperty(PersistedOptionValue.JSON_FLOAT_VAL) Double float_val) {
+      this.type = type;
+      this.kind = kind;
+      this.name = name;
+      this.num_val = num_val;
+      this.string_val = string_val;
+      this.bool_val = bool_val;
+      this.float_val = float_val;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) {
+        return true;
+      }
+
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
+
+      MockPersistedOptionValue that = (MockPersistedOptionValue) o;
+
+      if (!type.equals(that.type)) {
+        return false;
+      }
+
+      if (kind != that.kind) {
+        return false;
+      }
+
+      if (!name.equals(that.name)) {
+        return false;
+      }
+
+      if (num_val != null ? !num_val.equals(that.num_val) : that.num_val != null) {
+        return false;
+      }
+
+      if (string_val != null ? !string_val.equals(that.string_val) : that.string_val != null) {
+        return false;
+      }
+
+      if (bool_val != null ? !bool_val.equals(that.bool_val) : that.bool_val != null) {
+        return false;
+      }
+
+      return float_val != null ? float_val.equals(that.float_val) : that.float_val == null;
+    }
+
+    @Override
+    public int hashCode() {
+      int result = type.hashCode();
+      result = 31 * result + kind.hashCode();
+      result = 31 * result + name.hashCode();
+      result = 31 * result + (num_val != null ? num_val.hashCode() : 0);
+      result = 31 * result + (string_val != null ? string_val.hashCode() : 0);
+      result = 31 * result + (bool_val != null ? bool_val.hashCode() : 0);
+      result = 31 * result + (float_val != null ? float_val.hashCode() : 0);
+      return result;
+    }
+
+    @Override
+    public String toString() {
+      return "MockPersistedOptionValue{" + "type='" + type + '\'' + ", kind=" + kind + ", name='" + name +
+        '\'' + ", num_val=" + num_val + ", string_val='" + string_val + '\'' + ", bool_val=" + bool_val +
+        ", float_val=" + float_val + '}';
+    }
+  }
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestPStoreProviders.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestPStoreProviders.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestPStoreProviders.java
index 0504bb7..79a0f45 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestPStoreProviders.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/sys/TestPStoreProviders.java
@@ -15,21 +15,40 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.apache.drill.exec.store.sys;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.CuratorFrameworkFactory;
 import org.apache.curator.retry.RetryNTimes;
 import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.util.FileUtils;
 import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.exec.TestWithZookeeper;
+import org.apache.drill.exec.coord.zk.PathUtils;
+import org.apache.drill.exec.coord.zk.ZookeeperClient;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.exec.server.options.PersistedOptionValue;
 import org.apache.drill.exec.store.sys.store.provider.LocalPersistentStoreProvider;
 import org.apache.drill.exec.store.sys.store.provider.ZookeeperPersistentStoreProvider;
+import org.apache.drill.test.ClientFixture;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.FixtureBuilder;
+import org.apache.drill.testutils.DirTestWatcher;
+import org.apache.zookeeper.CreateMode;
+import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
 
+import java.io.File;
+
 public class TestPStoreProviders extends TestWithZookeeper {
   static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestPStoreProviders.class);
 
+  @Rule
+  public DirTestWatcher dirTestWatcher = new DirTestWatcher();
+
   @Test
   public void verifyLocalStore() throws Exception {
     try(LocalPersistentStoreProvider provider = new LocalPersistentStoreProvider(DrillConfig.create())){
@@ -39,19 +58,103 @@ public class TestPStoreProviders extends TestWithZookeeper {
 
   @Test
   public void verifyZkStore() throws Exception {
+    try(CuratorFramework curator = createCurator()){
+      curator.start();
+      ZookeeperPersistentStoreProvider provider = new ZookeeperPersistentStoreProvider(zkHelper.getConfig(), curator);
+      PStoreTestUtil.test(provider);
+    }
+  }
+
+  /**
+   * DRILL-5809
+   * Note: If this test breaks you are probably breaking backward and forward compatibility. Verify with the community
+   * that breaking compatibility is acceptable and planned for.
+   * @throws Exception
+   */
+  @Test
+  public void localBackwardCompatabilityTest() throws Exception {
+    localLoadTestHelper("src/test/resources/options/store/local/old/sys.options");
+  }
+
+  private void localLoadTestHelper(String propertiesDir) throws Exception {
+    File localOptionsResources = new File(propertiesDir);
+
+    String optionsDirPath = String.format("%s/sys.options", dirTestWatcher.getDirPath());
+    File optionsDir = new File(optionsDirPath);
+    optionsDir.mkdirs();
+
+    org.apache.commons.io.FileUtils.copyDirectory(localOptionsResources, optionsDir);
+
+    FixtureBuilder builder = ClusterFixture.builder().
+      configProperty(ExecConstants.HTTP_ENABLE, false).
+      configProperty(ExecConstants.SYS_STORE_PROVIDER_CLASS, LocalPersistentStoreProvider.class.getCanonicalName()).
+      configProperty(ExecConstants.SYS_STORE_PROVIDER_LOCAL_PATH, String.format("file://%s", dirTestWatcher.getDirPath())).
+      configProperty(ExecConstants.SYS_STORE_PROVIDER_LOCAL_ENABLE_WRITE, true);
+
+    try (ClusterFixture cluster = builder.build();
+         ClientFixture client = cluster.clientFixture()) {
+      String parquetPushdown = client.queryBuilder().
+        sql("SELECT val FROM sys.%s where name='%s'",
+          SystemTable.OPTION_VAL.getTableName(),
+          PlannerSettings.PARQUET_ROWGROUP_FILTER_PUSHDOWN_PLANNING_THRESHOLD_KEY).
+        singletonString();
+
+      String plannerWidth = client.queryBuilder().
+        sql("SELECT val FROM sys.%s where name='%s'",
+          SystemTable.OPTION_VAL.getTableName(),
+          ExecConstants.MAX_WIDTH_GLOBAL_KEY).
+        singletonString();
+
+      Assert.assertEquals("30000", parquetPushdown);
+      Assert.assertEquals("3333", plannerWidth);
+    }
+  }
+
+  /**
+   * DRILL-5809
+   * Note: If this test breaks you are probably breaking backward and forward compatibility. Verify with the community
+   * that breaking compatibility is acceptable and planned for.
+   * @throws Exception
+   */
+  @Test
+  public void zkBackwardCompatabilityTest() throws Exception {
+    final String oldName = "myOldOption";
+
+    try (CuratorFramework curator = createCurator()) {
+      curator.start();
+
+      PersistentStoreConfig<PersistedOptionValue> storeConfig =
+        PersistentStoreConfig.newJacksonBuilder(new ObjectMapper(), PersistedOptionValue.class).name("sys.test").build();
+
+      try (ZookeeperClient zkClient = new ZookeeperClient(curator,
+        PathUtils.join("/", storeConfig.getName()), CreateMode.PERSISTENT)) {
+        zkClient.start();
+        String oldOptionJson = FileUtils.getResourceAsString("/options/old_booleanopt.json");
+        zkClient.put(oldName, oldOptionJson.getBytes(), null);
+      }
+
+      try (ZookeeperPersistentStoreProvider provider =
+        new ZookeeperPersistentStoreProvider(zkHelper.getConfig(), curator)) {
+        PersistentStore<PersistedOptionValue> store = provider.getOrCreateStore(storeConfig);
+
+        PersistedOptionValue oldOptionValue = store.get(oldName, null);
+        PersistedOptionValue expectedValue = new PersistedOptionValue("true");
+
+        Assert.assertEquals(expectedValue, oldOptionValue);
+      }
+    }
+  }
+
+  private CuratorFramework createCurator() {
     String connect = zkHelper.getConnectionString();
     DrillConfig config = zkHelper.getConfig();
 
     CuratorFrameworkFactory.Builder builder = CuratorFrameworkFactory.builder()
-    .namespace(zkHelper.getConfig().getString(ExecConstants.ZK_ROOT))
-    .retryPolicy(new RetryNTimes(1, 100))
-    .connectionTimeoutMs(config.getInt(ExecConstants.ZK_TIMEOUT))
-    .connectString(connect);
+      .namespace(zkHelper.getConfig().getString(ExecConstants.ZK_ROOT))
+      .retryPolicy(new RetryNTimes(1, 100))
+      .connectionTimeoutMs(config.getInt(ExecConstants.ZK_TIMEOUT))
+      .connectString(connect);
 
-    try(CuratorFramework curator = builder.build()){
-      curator.start();
-      ZookeeperPersistentStoreProvider provider = new ZookeeperPersistentStoreProvider(config, curator);
-      PStoreTestUtil.test(provider);
-    }
+    return builder.build();
   }
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestCountDownLatchInjection.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestCountDownLatchInjection.java b/exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestCountDownLatchInjection.java
index 44cc3a7..407cb7c 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestCountDownLatchInjection.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestCountDownLatchInjection.java
@@ -122,7 +122,7 @@ public class TestCountDownLatchInjection extends BaseTestQuery {
 
   @Test // test would hang if the correct init, wait and countdowns did not happen, and the test timeout mechanism will
   // catch that case
-  public void latchInjected() {
+  public void latchInjected() throws InterruptedException {
     final int threads = 10;
     final ExtendedLatch trigger = new ExtendedLatch(1);
     final Pointer<Long> countingDownTime = new Pointer<>();
@@ -144,6 +144,11 @@ public class TestCountDownLatchInjection extends BaseTestQuery {
       fail("Thread should not be interrupted; there is no deliberate attempt.");
       return;
     }
+
+    while (countingDownTime.value == null) {
+      Thread.sleep(100L);
+    }
+
     assertTrue(timeSpentWaiting >= countingDownTime.value);
     try {
       queryContext.close();

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/java/org/apache/drill/exec/util/MiniZooKeeperCluster.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/util/MiniZooKeeperCluster.java b/exec/java-exec/src/test/java/org/apache/drill/exec/util/MiniZooKeeperCluster.java
index f9abe7f..d0359e1 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/util/MiniZooKeeperCluster.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/util/MiniZooKeeperCluster.java
@@ -154,8 +154,6 @@ public class MiniZooKeeperCluster {
       NIOServerCnxnFactory standaloneServerFactory;
 
       while (true) {
-        System.out.println("Starting zookeeper " + tentativePort);
-
         try {
           standaloneServerFactory = new NIOServerCnxnFactory();
           standaloneServerFactory.configure(new InetSocketAddress(tentativePort), 1000);
@@ -171,7 +169,7 @@ public class MiniZooKeeperCluster {
         try {
           standaloneServerFactory.startup(server);
         } catch (IOException e) {
-          LOG.error("Zookeeper startupt error", e);
+          LOG.error("Zookeeper startup error", e);
           tentativePort++;
           continue;
         }

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/resources/options/old_booleanopt.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/options/old_booleanopt.json b/exec/java-exec/src/test/resources/options/old_booleanopt.json
new file mode 100644
index 0000000..5614f5f
--- /dev/null
+++ b/exec/java-exec/src/test/resources/options/old_booleanopt.json
@@ -0,0 +1,6 @@
+{
+  "kind": "BOOLEAN",
+  "name": "myOldOption",
+  "bool_val": true,
+  "type": "SYSTEM"
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/resources/options/old_doubleopt.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/options/old_doubleopt.json b/exec/java-exec/src/test/resources/options/old_doubleopt.json
new file mode 100644
index 0000000..08d3924
--- /dev/null
+++ b/exec/java-exec/src/test/resources/options/old_doubleopt.json
@@ -0,0 +1,6 @@
+{
+  "kind": "DOUBLE",
+  "name": "myOldOption",
+  "float_val": 1.5,
+  "type": "SYSTEM"
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/resources/options/old_longopt.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/options/old_longopt.json b/exec/java-exec/src/test/resources/options/old_longopt.json
new file mode 100644
index 0000000..cd55173
--- /dev/null
+++ b/exec/java-exec/src/test/resources/options/old_longopt.json
@@ -0,0 +1,6 @@
+{
+  "kind": "LONG",
+  "name": "myOldOption",
+  "num_val": 5000,
+  "type": "SYSTEM"
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/resources/options/old_stringopt.json
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/options/old_stringopt.json b/exec/java-exec/src/test/resources/options/old_stringopt.json
new file mode 100644
index 0000000..c4fe699
--- /dev/null
+++ b/exec/java-exec/src/test/resources/options/old_stringopt.json
@@ -0,0 +1,6 @@
+{
+  "kind": "STRING",
+  "name": "myOldOption",
+  "string_val": "wabalubadubdub",
+  "type": "SYSTEM"
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/resources/options/store/local/old/sys.options/planner.store.parquet.rowgroup.filter.pushdown.threshold.sys.drill
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/options/store/local/old/sys.options/planner.store.parquet.rowgroup.filter.pushdown.threshold.sys.drill b/exec/java-exec/src/test/resources/options/store/local/old/sys.options/planner.store.parquet.rowgroup.filter.pushdown.threshold.sys.drill
new file mode 100644
index 0000000..8d89a59
--- /dev/null
+++ b/exec/java-exec/src/test/resources/options/store/local/old/sys.options/planner.store.parquet.rowgroup.filter.pushdown.threshold.sys.drill
@@ -0,0 +1,6 @@
+{
+  "kind" : "LONG",
+  "type" : "SYSTEM",
+  "name" : "planner.store.parquet.rowgroup.filter.pushdown.threshold",
+  "num_val" : 30000
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/exec/java-exec/src/test/resources/options/store/local/old/sys.options/planner.width.max_per_query.sys.drill
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/resources/options/store/local/old/sys.options/planner.width.max_per_query.sys.drill b/exec/java-exec/src/test/resources/options/store/local/old/sys.options/planner.width.max_per_query.sys.drill
new file mode 100644
index 0000000..186d736
--- /dev/null
+++ b/exec/java-exec/src/test/resources/options/store/local/old/sys.options/planner.width.max_per_query.sys.drill
@@ -0,0 +1,6 @@
+{
+  "kind" : "LONG",
+  "type" : "SYSTEM",
+  "name" : "planner.width.max_per_query",
+  "num_val" : 3333
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/drill/blob/8a8bf63f/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 4f8ec67..e64c282 100644
--- a/pom.xml
+++ b/pom.xml
@@ -232,6 +232,7 @@
             <exclude>**/*.httpd</exclude>
             <exclude>**/*.autotools</exclude>
             <exclude>**/*.cproject</exclude>
+            <exclude>**/*.drill</exclude>
             <!-- TODO DRILL-4336: try to avoid the need to add this -->
             <exclude>dependency-reduced-pom.xml</exclude>
           </excludes>


Mime
View raw message