You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2026/01/15 21:00:02 UTC
[PR] Fixes to MiniAccumulo based on comments in #5999 [accumulo]
dlmarion opened a new pull request, #6062:
URL: https://github.com/apache/accumulo/pull/6062
(no comment)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixes to MiniAccumulo based on comments in #5999 [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #6062:
URL: https://github.com/apache/accumulo/pull/6062
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fixes to MiniAccumulo based on comments in #5999 [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #6062:
URL: https://github.com/apache/accumulo/pull/6062#discussion_r2695983497
##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java:
##########
@@ -347,10 +347,18 @@ private ProcessInfo _exec(Class<?> clazz, List<String> extraJvmOpts, String... a
String javaHome = System.getProperty("java.home");
String javaBin = javaHome + File.separator + "bin" + File.separator + "java";
+ Stream<String> defaultJvmOpts = Stream.of("-XX:+PerfDisableSharedMem", "-XX:+AlwaysPreTouch");
+ Map<String,String> defaultSystemProps =
+ Map.of("-Dapple.awt.UIElement", "true", "-Djava.net.preferIPv4Stack", "true");
+
var basicArgs = Stream.of(javaBin, "-Dproc=" + clazz.getSimpleName());
- var jvmOptions = Stream.concat(config.getJvmOptions().stream(), extraJvmOpts.stream());
- var systemProps = config.getSystemProperties().entrySet().stream()
- .map(e -> String.format("-D%s=%s", e.getKey(), e.getValue()));
+
+ var jvmOptions = Stream.concat(Stream.concat(defaultJvmOpts, config.getJvmOptions().stream()),
+ extraJvmOpts.stream());
+ var systemProps = Stream
+ .concat(defaultSystemProps.entrySet().stream(),
+ config.getSystemProperties().entrySet().stream())
+ .map(e -> String.format("%s=%s", e.getKey(), e.getValue()));
Review Comment:
This drops the `-D`, but it means that users need to add it to their keys when they put it in their map, as in:
`cfg.setSystemProperties(Map.of("-DsomeKey", "someValue));`
That doesn't quite make sense. They should be prepended to the key=value when the command-line is being constructed, not expected to be added to the key by the user. The redundant one can be dropped from the defaults instead.
##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java:
##########
@@ -347,10 +347,18 @@ private ProcessInfo _exec(Class<?> clazz, List<String> extraJvmOpts, String... a
String javaHome = System.getProperty("java.home");
String javaBin = javaHome + File.separator + "bin" + File.separator + "java";
+ Stream<String> defaultJvmOpts = Stream.of("-XX:+PerfDisableSharedMem", "-XX:+AlwaysPreTouch");
Review Comment:
This is fine since the stream isn't reused, but it might be better to put these in a `Set.of` outside the method, and call `.stream()` inside the method, to reuse the set.
However, I don't even know why these are here as default options. There's no documentation explaining why they were added. They were added in https://issues.apache.org/jira/browse/ACCUMULO-3699, but no specific explanation about what they were expected to achieve. They may be OBE.
##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloConfigImpl.java:
##########
@@ -677,7 +672,7 @@ public File getClientPropsFile() {
* @since 1.6.0
*/
public void setSystemProperties(Map<String,String> systemProperties) {
- this.systemProperties.putAll(systemProperties);
+ this.systemProperties = new HashMap<>(systemProperties);
Review Comment:
👍
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org