Ethanlm commented on a change in pull request #3319:
URL: https://github.com/apache/storm/pull/3319#discussion_r481351414
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1907,4 +1909,201 @@ private void readArchive(ZipFile zipFile) throws
IOException {
}
}
}
+
+ /**
+ * Return path to the Java command "x", prefixing with $}{JAVA_HOME}/bin/
if JAVA_HOME system property is defined.
+ * Otherwie return the supplied Java command unmodified.
+ *
+ * @param cmd Java command, e.g. "java", "jar" etc.
+ * @return command string to use.
+ */
+ public static String getJavaCmd(String cmd) {
+
+ String ret = null;
+ String javaHome = System.getenv().get("JAVA_HOME");
+ if (StringUtils.isNotBlank(javaHome)) {
+ ret = javaHome + File.separator + "bin" + File.separator + cmd;
+ } else {
+ ret = cmd;
+ }
+ return ret;
+ }
+
+ /**
+ * Find the list of substitutable variable names in supplied string.
Substitutable variable names are
+ * preceded and followed by a percent sign and composed of upper case
characters, numbers, underscore and dash
+ * characters.
+ *
+ * @param str string with 0 or more substitutable variables.
+ * @return a set with 0 or more variable names.
+ */
+ public static Set<String> findSubstitutableVarNames(String str) {
+ final String patternStr = "%([A-Z0-9_-]+)%";
+ final int matchedGroup = 1; // matched group is 1 because of the
capturing parens in the patternStr
+
+ Set<String> ret = new TreeSet<>();
+ Pattern p = Pattern.compile(patternStr);
+ Matcher m = p.matcher(str);
+ int matchCnt = 0;
+ while (m.find()) {
+ matchCnt++;
+ try {
+ ret.add(str.substring(m.start(matchedGroup),
m.end(matchedGroup)));
+ } catch (IllegalStateException ex) {
+ String err = String.format("Internal Error in
findSubstitutableVarNames(\"%s\"), pattern \"%s\", matchCnt=%d",
+ str, patternStr, matchCnt, str);
+ LOG.error(err, ex);
+ }
+ }
+ return ret;
+ }
+
+ /**
+ * Perform variable substitutions using the supplied substitutions.
+ * Leave unchanged any placeholders for undefined variables.
+ *
+ * @param str original string with placeholders for replacements.
+ * @param substitutions map of substitution variables and values.
+ * @return final string with variable replaced with supplied substitutions.
+ */
+ public static String substituteVarNames(String str, Map<String, Object>
substitutions) {
+ Set<String> vars = findSubstitutableVarNames(str);
+ for (String var : vars) {
+ if (!substitutions.containsKey(var)) {
+ continue;
+ }
+ String orig = "%" + var + "%";
+ String substitution = String.valueOf(substitutions.get(var));
+ str = str.replace(orig, substitution);
+ }
+ return str;
+ }
+
+ public static List<String> substituteVarNamesInObject(Object value,
Map<String, Object> substitutions) {
+ List<String> rets = new ArrayList<>();
+ if (value instanceof String) {
+ String string = substituteVarNames((String) value, substitutions);
+ if (StringUtils.isNotBlank(string)) {
+ rets.add(string);
+ }
+ } else if (value instanceof List) {
+ ((List<String>) value).forEach(x -> {
+ x = substituteVarNames(x, substitutions);
+ if (StringUtils.isNotBlank(x)) {
+ rets.add(x);
+ }
+ });
+ }
+ return rets;
+ }
+
+ /**
+ * Enumeration of variables that can be substituted in Java command string.
+ */
+ public enum WellKnownRuntimeSubstitutionVars {
+ ID("ID"),
+ WORKER_ID("WORKER-ID"),
+ TOPOLOGY_ID("TOPOLOGY-ID"),
+ WORKER_PORT("WORKER-PORT"),
+ HEAP_MEM("HEAP-MEM"),
+ OFF_HEAP_MEM("OFF-HEAP-MEM"),
+ LIMIT_MEM("LIMIT-MEM");
+
+ private String varName;
+
+ WellKnownRuntimeSubstitutionVars(String varName) {
+ this.varName = varName;
+ }
+
+ public String getVarName() {
+ return varName;
+ }
+
+ public static Map<String, Object> getDummySubstitutions() {
+ Map<String, Object> ret = new HashMap<>();
+ ret.put(ID.getVarName(), "dummyId");
+ ret.put(WORKER_ID.getVarName(), "dummy-worker-id");
+ ret.put(TOPOLOGY_ID.getVarName(), "dummy-topology-id");
+ ret.put(WORKER_PORT.getVarName(), 6700);
+ ret.put(HEAP_MEM.getVarName(), 1024);
+ ret.put(OFF_HEAP_MEM.getVarName(), 1024);
+ ret.put(LIMIT_MEM.getVarName(), 1024);
+ return ret;
+ }
+ }
+
+ /**
+ * Launch a validation java command (java -showversion
java.util.prefs.Base64 1 1) with JVM options
+ * used in worker launch to validate JVM options.
+ *
+ * @param supervisorConf configuration for the supervisor. May be null.
+ * @param topoConf configuration for the topology. Must be provided.
+ * @param substitutions may be null in which case it is {@link
WellKnownRuntimeSubstitutionVars#getDummySubstitutions()}
+ * @param throwExceptionOnFailure if true then an exception is thrown
instead of returning false
+ * @return true if the option combination is valid, false otherwise (or
throws InvalidTopologyException)
+ */
+ public static boolean validateWorkerLaunchOptions(Map<String, Object>
supervisorConf, Map<String, Object> topoConf,
+ Map<String, Object>
substitutions, boolean throwExceptionOnFailure)
+ throws InvalidTopologyException {
+ // from storm-server/.../BasicContainer.mkLaunchCommand
Review comment:
I don't like the replication either. It adds a lot of maintenance
overhead. The suggestion I can think of is to move the whole check into server
side at `Nimbus.submitTopologyWithOpts`, which is better in a few ways. But it
has its own problem to be taken care of (see my comments above).
Some thoughts on this:
This feature (check GC option conflicts) is nice to have. But it is not hard
to find out that workers fail because of GC option conflicts. So I think we
don't have to implement this feature if there is no good/clean way to implement
it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
