This is an automated email from the ASF dual-hosted git repository.
jmanno pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new a8db761 Closes #1929. Add property verification for Sampler Options
(#1944)
a8db761 is described below
commit a8db76146740770fe86a84ff5428b15f5c8e7e68
Author: Jeffrey Manno <jeffreymann...@gmail.com>
AuthorDate: Thu Feb 25 13:12:40 2021 -0500
Closes #1929. Add property verification for Sampler Options (#1944)
Co-authored-by: Christopher Tubbs <ctubb...@apache.org>
---
.../core/client/sample/AbstractHashSampler.java | 29 ++++++++++++++++++----
.../core/client/sample/RowColumnSampler.java | 10 ++++++--
.../accumulo/core/client/sample/Sampler.java | 8 ++++++
.../accumulo/core/sample/impl/SamplerFactory.java | 13 +++++++---
4 files changed, 49 insertions(+), 11 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/client/sample/AbstractHashSampler.java
b/core/src/main/java/org/apache/accumulo/core/client/sample/AbstractHashSampler.java
index df22797..8f5f29c 100644
---
a/core/src/main/java/org/apache/accumulo/core/client/sample/AbstractHashSampler.java
+++
b/core/src/main/java/org/apache/accumulo/core/client/sample/AbstractHashSampler.java
@@ -23,6 +23,7 @@ import static java.util.Objects.requireNonNull;
import java.io.DataOutput;
import java.io.IOException;
+import java.util.Map;
import java.util.Set;
import org.apache.accumulo.core.data.Key;
@@ -57,12 +58,34 @@ public abstract class AbstractHashSampler implements
Sampler {
private int modulus;
private static final Set<String> VALID_OPTIONS = Set.of("hasher", "modulus");
+ private static final Set<String> VALID_VALUES_HASHER = Set.of("murmur3_32",
"md5", "sha1");
+
+ /**
+ * Subclasses with options should override this method to validate subclass
options while also
+ * calling {@code super.validateOptions(config)} to validate base class
options.
+ */
+ @Override
+ public void validateOptions(Map<String,String> config) {
+ for (Map.Entry<String,String> entry : config.entrySet()) {
+ checkArgument(VALID_OPTIONS.contains(entry.getKey()), "Unknown option:
%s", entry.getKey());
+
+ if (entry.getKey().equals("hasher"))
+ checkArgument(VALID_VALUES_HASHER.contains(entry.getValue()),
+ "Unknown value for hasher: %s", entry.getValue());
+
+ if (entry.getKey().equals("modulus"))
+ checkArgument(Integer.parseInt(entry.getValue()) > 0,
+ "Improper Integer value for modulus: %s", entry.getValue());
+ }
+ }
/**
* Subclasses with options should override this method and return true if
the option is valid for
* the subclass or if {@code super.isValidOption(opt)} returns true.
+ *
+ * @deprecated since 2.1.0, replaced by {@link #validateOptions(Map)}
*/
-
+ @Deprecated(since = "2.1.0")
protected boolean isValidOption(String option) {
return VALID_OPTIONS.contains(option);
}
@@ -80,10 +103,6 @@ public abstract class AbstractHashSampler implements
Sampler {
requireNonNull(hasherOpt, "Hasher not specified");
requireNonNull(modulusOpt, "Modulus not specified");
- for (String option : config.getOptions().keySet()) {
- checkArgument(isValidOption(option), "Unknown option : %s", option);
- }
-
switch (hasherOpt) {
case "murmur3_32":
hashFunction = Hashing.murmur3_32();
diff --git
a/core/src/main/java/org/apache/accumulo/core/client/sample/RowColumnSampler.java
b/core/src/main/java/org/apache/accumulo/core/client/sample/RowColumnSampler.java
index a2785e3..1d71214 100644
---
a/core/src/main/java/org/apache/accumulo/core/client/sample/RowColumnSampler.java
+++
b/core/src/main/java/org/apache/accumulo/core/client/sample/RowColumnSampler.java
@@ -18,8 +18,11 @@
*/
package org.apache.accumulo.core.client.sample;
+import static com.google.common.base.Preconditions.checkArgument;
+
import java.io.DataOutput;
import java.io.IOException;
+import java.util.Map;
import java.util.Set;
import org.apache.accumulo.core.data.ByteSequence;
@@ -80,8 +83,11 @@ public class RowColumnSampler extends AbstractHashSampler {
}
@Override
- protected boolean isValidOption(String option) {
- return super.isValidOption(option) || VALID_OPTIONS.contains(option);
+ public void validateOptions(Map<String,String> config) {
+ super.validateOptions(config);
+ for (String option : config.keySet()) {
+ checkArgument(VALID_OPTIONS.contains(option), "Unknown option : %s",
option);
+ }
}
@Override
diff --git
a/core/src/main/java/org/apache/accumulo/core/client/sample/Sampler.java
b/core/src/main/java/org/apache/accumulo/core/client/sample/Sampler.java
index d7e07c8..810b8ae 100644
--- a/core/src/main/java/org/apache/accumulo/core/client/sample/Sampler.java
+++ b/core/src/main/java/org/apache/accumulo/core/client/sample/Sampler.java
@@ -18,6 +18,8 @@
*/
package org.apache.accumulo.core.client.sample;
+import java.util.Map;
+
import org.apache.accumulo.core.data.Key;
/**
@@ -61,4 +63,10 @@ public interface Sampler {
* Return false if it should not be included.
*/
boolean accept(Key k);
+
+ /**
+ * @param config
+ * Sampler options configuration to validate. Validates option and
value.
+ */
+ default void validateOptions(Map<String,String> config) {}
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java
b/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java
index f35d005..8f23314 100644
---
a/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java
+++
b/core/src/main/java/org/apache/accumulo/core/sample/impl/SamplerFactory.java
@@ -23,10 +23,14 @@ import java.io.IOException;
import org.apache.accumulo.core.classloader.ClassLoaderUtil;
import org.apache.accumulo.core.client.sample.Sampler;
import org.apache.accumulo.core.conf.AccumuloConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class SamplerFactory {
+ private static final Logger log =
LoggerFactory.getLogger(SamplerFactory.class);
+
public static Sampler newSampler(SamplerConfigurationImpl config,
AccumuloConfiguration acuconf,
- boolean useAccumuloStart) throws IOException {
+ boolean useAccumuloStart) {
String context = ClassLoaderUtil.tableContext(acuconf);
Class<? extends Sampler> clazz;
@@ -38,13 +42,14 @@ public class SamplerFactory {
clazz = ClassLoaderUtil.loadClass(context, config.getClassName(),
Sampler.class);
Sampler sampler = clazz.getDeclaredConstructor().newInstance();
-
+ sampler.validateOptions(config.getOptions());
sampler.init(config.toSamplerConfiguration());
return sampler;
- } catch (ReflectiveOperationException e) {
- throw new RuntimeException(e);
+ } catch (ReflectiveOperationException | RuntimeException e) {
+ log.error("Cannot initialize sampler {}: {}", config.getClassName(),
e.getMessage(), e);
+ return null;
}
}
