From 539e11e062aff768adba1526539a130321a0b871 Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Thu, 11 Dec 2014 17:47:17 +0900 Subject: [PATCH 1/2] TAJO-1244: tajo.worker.tmpdir.locations should use a validator for a list of Path. --- .../java/org/apache/tajo/conf/TajoConf.java | 3 +- .../tajo/validation/PathListValidator.java | 63 +++++++++++++++++++ .../apache/tajo/validation/PathValidator.java | 4 +- .../apache/tajo/validation/Validators.java | 4 ++ .../tajo/validation/TestValidators.java | 26 ++++++++ 5 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 tajo-common/src/main/java/org/apache/tajo/validation/PathListValidator.java diff --git a/tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java b/tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java index a38bd6c2c9..252b8f051a 100644 --- a/tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java +++ b/tajo-common/src/main/java/org/apache/tajo/conf/TajoConf.java @@ -181,8 +181,7 @@ public static enum ConfVars implements ConfigKey { WORKER_QM_RPC_ADDRESS("tajo.worker.qm-rpc.address", "0.0.0.0:28093", Validators.networkAddr()), // Tajo Worker Temporal Directories - WORKER_TEMPORAL_DIR("tajo.worker.tmpdir.locations", "/tmp/tajo-${user.name}/tmpdir", - Validators.pathUrl()), + WORKER_TEMPORAL_DIR("tajo.worker.tmpdir.locations", "/tmp/tajo-${user.name}/tmpdir", Validators.pathUrlList()), WORKER_TEMPORAL_DIR_CLEANUP("tajo.worker.tmpdir.cleanup-at-startup", false, Validators.bool()), // Tajo Worker Resources diff --git a/tajo-common/src/main/java/org/apache/tajo/validation/PathListValidator.java b/tajo-common/src/main/java/org/apache/tajo/validation/PathListValidator.java new file mode 100644 index 0000000000..8902b5eef5 --- /dev/null +++ b/tajo-common/src/main/java/org/apache/tajo/validation/PathListValidator.java @@ -0,0 +1,63 @@ +/** + * 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.tajo.validation; + +import org.apache.tajo.util.TUtil; + +import java.util.Collection; + +public class PathListValidator extends AbstractValidator { + private static final String LIST_SEPARATOR = ","; + + @Override + protected String getErrorMessage(T object) { + return object + " is not valid path."; + } + + @Override + protected boolean validateInternal(T object) { + PathValidator validator = (PathValidator) Validators.pathUrl(); + + boolean result = true; + + if (object != null) { + if (object instanceof CharSequence) { + String valueString = object.toString(); + if (valueString.isEmpty()) { + result = true; + } else { + String [] splits = object.toString().split(LIST_SEPARATOR); + for (String path : splits) { + result &= validator.validateInternal(path.trim()); + } + } + } + } else { + result = true; + } + + return result; + } + + @Override + protected Collection getDependantValidators() { + return TUtil.newList(); + } + +} diff --git a/tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java b/tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java index a76587cd69..9548e80522 100644 --- a/tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java +++ b/tajo-common/src/main/java/org/apache/tajo/validation/PathValidator.java @@ -19,9 +19,11 @@ package org.apache.tajo.validation; public class PathValidator extends PatternValidator { + static final String PATH_REGEXP_PATTERN = + "^(?:[a-zA-Z][a-zA-Z0-9+-.]+:[/]{1,2}[a-zA-Z-.]*[:0-9]*)?(?:/?[a-zA-Z]:)?[/a-zA-Z0-9-_\\\\.\\\\$\\\\{\\\\}]*$"; public PathValidator() { - super("^(?:[a-zA-Z][a-zA-Z0-9+-.]+:[/]{1,2}[a-zA-Z-.]*[:0-9]*)?(?:/?[a-zA-Z]:)?[/a-zA-Z0-9-_\\\\.\\\\$\\\\{\\\\}]*$"); + super(PATH_REGEXP_PATTERN); } @Override diff --git a/tajo-common/src/main/java/org/apache/tajo/validation/Validators.java b/tajo-common/src/main/java/org/apache/tajo/validation/Validators.java index acd4876a7e..152c41e946 100644 --- a/tajo-common/src/main/java/org/apache/tajo/validation/Validators.java +++ b/tajo-common/src/main/java/org/apache/tajo/validation/Validators.java @@ -53,6 +53,10 @@ public static Validator range(String minValue, String maxValue) { public static Validator pathUrl() { return new PathValidator(); } + + public static Validator pathUrlList() { + return new PathValidator(); + } public static Validator shellVar() { return new ShellVariableValidator(); diff --git a/tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java b/tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java index 16ce49f87f..060f19287a 100644 --- a/tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java +++ b/tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java @@ -397,6 +397,32 @@ public void testPathValidator() { assertThat(new PathValidator().validate(invalidUrl), hasItem(hasAClass(equalTo(PathValidator.class)))); } + + @Test + public void testPathListValidator() { + String validUrls = + "file:///tmp/tajo-$root/a,file:///tmp/tajo-${user.name}/,file:/home/tajo/test-data/TestExternalSortExec"; + assertThat(new PathListValidator().validateInternal(validUrls), is(true)); + assertThat(new PathListValidator().validate(validUrls).size(), is(0)); + + validUrls = + "file:///tmp/tajo-$root/a, file:///tmp/tajo-${user.name}/, file:/home/tajo/test-data/TestExternalSortExec"; + assertThat(new PathListValidator().validateInternal(validUrls), is(true)); + assertThat(new PathListValidator().validate(validUrls).size(), is(0)); + + String invalidUrls = + "file:///tmp/tajo-$root/a, t!ef:///tmp/tajo-root, file:/home/tajo/test-data/TestExternalSortExec"; + assertThat(new PathValidator().validateInternal(invalidUrls), is(false)); + assertThat(new PathValidator().validate(invalidUrls).size(), is(1)); + assertThat(new PathValidator().validate(invalidUrls), + hasItem(hasAClass(equalTo(PathValidator.class)))); + + invalidUrls = "This is not a valid url,This is not a valid url"; + assertThat(new PathValidator().validateInternal(invalidUrls), is(false)); + assertThat(new PathValidator().validate(invalidUrls).size(), is(1)); + assertThat(new PathValidator().validate(invalidUrls), + hasItem(hasAClass(equalTo(PathValidator.class)))); + } @Test public void testShellVariableValidator() { From 5a60ffec80cf4594b9fc9dfb02afc70a39abafc8 Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Thu, 11 Dec 2014 18:52:50 +0900 Subject: [PATCH 2/2] Added some missed code and activation flag to rules. --- .../rule/SelfDiagnosisRuleDefinition.java | 3 ++- .../tajo/rule/SelfDiagnosisRuleEngine.java | 22 +++++++++++++------ .../tajo/validation/PathListValidator.java | 2 +- .../apache/tajo/validation/Validators.java | 2 +- .../tajo/validation/TestValidators.java | 20 ++++++++++------- .../ConnectivityCheckerRuleForTajoWorker.java | 3 ++- 6 files changed, 33 insertions(+), 19 deletions(-) diff --git a/tajo-common/src/main/java/org/apache/tajo/rule/SelfDiagnosisRuleDefinition.java b/tajo-common/src/main/java/org/apache/tajo/rule/SelfDiagnosisRuleDefinition.java index ae1d8ac464..210756c9be 100644 --- a/tajo-common/src/main/java/org/apache/tajo/rule/SelfDiagnosisRuleDefinition.java +++ b/tajo-common/src/main/java/org/apache/tajo/rule/SelfDiagnosisRuleDefinition.java @@ -32,5 +32,6 @@ public String name(); public int priority() default -1; - + + public boolean enabled() default true; } diff --git a/tajo-common/src/main/java/org/apache/tajo/rule/SelfDiagnosisRuleEngine.java b/tajo-common/src/main/java/org/apache/tajo/rule/SelfDiagnosisRuleEngine.java index 12bd4f6f86..df0225d0fa 100644 --- a/tajo-common/src/main/java/org/apache/tajo/rule/SelfDiagnosisRuleEngine.java +++ b/tajo-common/src/main/java/org/apache/tajo/rule/SelfDiagnosisRuleEngine.java @@ -65,14 +65,16 @@ protected Map> getRules() { private void loadRuleData(List ruleList) { for (SelfDiagnosisRule rule: ruleList) { RuleWrapper wrapper = new RuleWrapper(rule); - Map categoryMap = wrapperMap.get(wrapper.getCategoryName()); - - if (categoryMap == null) { - categoryMap = TUtil.newHashMap(); - wrapperMap.put(wrapper.getCategoryName(), categoryMap); + if (wrapper.isEnabled()) { + Map categoryMap = wrapperMap.get(wrapper.getCategoryName()); + + if (categoryMap == null) { + categoryMap = TUtil.newHashMap(); + wrapperMap.put(wrapper.getCategoryName(), categoryMap); + } + + categoryMap.put(wrapper.getRuleName(), wrapper); } - - categoryMap.put(wrapper.getRuleName(), wrapper); } } @@ -92,6 +94,7 @@ class RuleWrapper implements Comparable { private final String categoryName; private final String ruleName; private final int priority; + private final boolean enabled; private final Class[] acceptedCallers; private final SelfDiagnosisRule rule; @@ -105,6 +108,7 @@ public RuleWrapper(SelfDiagnosisRule rule) { categoryName = ruleDefinition.category(); ruleName = ruleDefinition.name(); priority = ruleDefinition.priority(); + enabled = ruleDefinition.enabled(); SelfDiagnosisRuleVisibility.LimitedPrivate limitedPrivateScope = rule.getClass().getAnnotation(SelfDiagnosisRuleVisibility.LimitedPrivate.class); @@ -137,6 +141,10 @@ public int getPriority() { return priority; } + public boolean isEnabled() { + return enabled; + } + @Override public int compareTo(RuleWrapper o) { if (getPriority() == -1 && o.getPriority() == -1) { diff --git a/tajo-common/src/main/java/org/apache/tajo/validation/PathListValidator.java b/tajo-common/src/main/java/org/apache/tajo/validation/PathListValidator.java index 8902b5eef5..a580cb5caa 100644 --- a/tajo-common/src/main/java/org/apache/tajo/validation/PathListValidator.java +++ b/tajo-common/src/main/java/org/apache/tajo/validation/PathListValidator.java @@ -27,7 +27,7 @@ public class PathListValidator extends AbstractValidator { @Override protected String getErrorMessage(T object) { - return object + " is not valid path."; + return object + " is not valid path list."; } @Override diff --git a/tajo-common/src/main/java/org/apache/tajo/validation/Validators.java b/tajo-common/src/main/java/org/apache/tajo/validation/Validators.java index 152c41e946..d4a3fa4cfa 100644 --- a/tajo-common/src/main/java/org/apache/tajo/validation/Validators.java +++ b/tajo-common/src/main/java/org/apache/tajo/validation/Validators.java @@ -55,7 +55,7 @@ public static Validator pathUrl() { } public static Validator pathUrlList() { - return new PathValidator(); + return new PathListValidator(); } public static Validator shellVar() { diff --git a/tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java b/tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java index 060f19287a..c539a51efb 100644 --- a/tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java +++ b/tajo-common/src/test/java/org/apache/tajo/validation/TestValidators.java @@ -410,18 +410,22 @@ public void testPathListValidator() { assertThat(new PathListValidator().validateInternal(validUrls), is(true)); assertThat(new PathListValidator().validate(validUrls).size(), is(0)); + validUrls = "/tmp/tajo-hyunsik/tmpdir1,/tmp/tajo-hyunsik/tmpdir2"; + assertThat(new PathListValidator().validateInternal(validUrls), is(true)); + assertThat(new PathListValidator().validate(validUrls).size(), is(0)); + String invalidUrls = "file:///tmp/tajo-$root/a, t!ef:///tmp/tajo-root, file:/home/tajo/test-data/TestExternalSortExec"; - assertThat(new PathValidator().validateInternal(invalidUrls), is(false)); - assertThat(new PathValidator().validate(invalidUrls).size(), is(1)); - assertThat(new PathValidator().validate(invalidUrls), - hasItem(hasAClass(equalTo(PathValidator.class)))); + assertThat(new PathListValidator().validateInternal(invalidUrls), is(false)); + assertThat(new PathListValidator().validate(invalidUrls).size(), is(1)); + assertThat(new PathListValidator().validate(invalidUrls), + hasItem(hasAClass(equalTo(PathListValidator.class)))); invalidUrls = "This is not a valid url,This is not a valid url"; - assertThat(new PathValidator().validateInternal(invalidUrls), is(false)); - assertThat(new PathValidator().validate(invalidUrls).size(), is(1)); - assertThat(new PathValidator().validate(invalidUrls), - hasItem(hasAClass(equalTo(PathValidator.class)))); + assertThat(new PathListValidator().validateInternal(invalidUrls), is(false)); + assertThat(new PathListValidator().validate(invalidUrls).size(), is(1)); + assertThat(new PathListValidator().validate(invalidUrls), + hasItem(hasAClass(equalTo(PathListValidator.class)))); } @Test diff --git a/tajo-core/src/main/java/org/apache/tajo/worker/rule/ConnectivityCheckerRuleForTajoWorker.java b/tajo-core/src/main/java/org/apache/tajo/worker/rule/ConnectivityCheckerRuleForTajoWorker.java index dcd40bf9ce..328a31bbe9 100644 --- a/tajo-core/src/main/java/org/apache/tajo/worker/rule/ConnectivityCheckerRuleForTajoWorker.java +++ b/tajo-core/src/main/java/org/apache/tajo/worker/rule/ConnectivityCheckerRuleForTajoWorker.java @@ -38,7 +38,8 @@ /** * With this rule, Tajo worker will check the connectivity to tajo master server. */ -@SelfDiagnosisRuleDefinition(category="worker", name="ConnectivityCheckerRuleForTajoWorker", priority=0) +@SelfDiagnosisRuleDefinition( + category="worker", name="ConnectivityCheckerRuleForTajoWorker", priority=0, enabled = false) @SelfDiagnosisRuleVisibility.LimitedPrivate(acceptedCallers = { TajoWorker.class }) public class ConnectivityCheckerRuleForTajoWorker implements SelfDiagnosisRule {