exceptionfactory commented on code in PR #7891:
URL: https://github.com/apache/nifi/pull/7891#discussion_r1366390227
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java:
##########
@@ -1392,6 +1404,33 @@ public boolean accept(final File dir, final String
filename) {
}
}
+ private String printBasicOSJavaDetails(final String xms, final String xmx)
{
+ Map<String, String> details = new HashMap<String, String>(6);
+
+ // java version
+ details.put("javaVersion", System.getProperty("java.version"));
+
+ // num cores
+ details.put("cores",
Integer.toString(Runtime.getRuntime().availableProcessors()));
+
+ // max file descriptor count + total physical memory
+ try {
+ final ObjectName osObjectName =
ManagementFactory.getOperatingSystemMXBean().getObjectName();
+ final Object maxOpenFileCount =
ManagementFactory.getPlatformMBeanServer().getAttribute(osObjectName,
"MaxFileDescriptorCount");
+ details.put("maxOpenFileDescriptors",
String.valueOf(maxOpenFileCount));
+ final Object totalPhysicalMemory =
ManagementFactory.getPlatformMBeanServer().getAttribute(osObjectName,
"TotalPhysicalMemorySize");
+ details.put("totalPhysicalMemoryMB", String.valueOf(((Long)
totalPhysicalMemory) / (1024*1024)));
Review Comment:
The return of `getPlatformMBeanServer()` should be declared once instead of
being called multiple times. For safety, the response of `getAttribute()`
should checked for `null` to avoid potential issues.
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java:
##########
@@ -1392,6 +1404,33 @@ public boolean accept(final File dir, final String
filename) {
}
}
+ private String printBasicOSJavaDetails(final String xms, final String xmx)
{
+ Map<String, String> details = new HashMap<String, String>(6);
+
+ // java version
+ details.put("javaVersion", System.getProperty("java.version"));
+
+ // num cores
+ details.put("cores",
Integer.toString(Runtime.getRuntime().availableProcessors()));
+
+ // max file descriptor count + total physical memory
+ try {
+ final ObjectName osObjectName =
ManagementFactory.getOperatingSystemMXBean().getObjectName();
+ final Object maxOpenFileCount =
ManagementFactory.getPlatformMBeanServer().getAttribute(osObjectName,
"MaxFileDescriptorCount");
+ details.put("maxOpenFileDescriptors",
String.valueOf(maxOpenFileCount));
+ final Object totalPhysicalMemory =
ManagementFactory.getPlatformMBeanServer().getAttribute(osObjectName,
"TotalPhysicalMemorySize");
+ details.put("totalPhysicalMemoryMB", String.valueOf(((Long)
totalPhysicalMemory) / (1024*1024)));
+ } catch (final Throwable t) {
+ // Ignore. This will throw either ClassNotFound or
NoClassDefFoundError if unavailable in this JVM.
+ }
+
+ // min/max heap
+ details.put("xms", xms);
+ details.put("xmx", xmx);
Review Comment:
What do you think of spelling out these values as `minimumHeapSize` and
`maximumHeapSize`?
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java:
##########
@@ -1165,13 +1168,22 @@ public void start(final boolean monitor) throws
IOException {
nifiPropsFilename = nifiPropsFilename.trim();
+ String xmx = null;
+ String xms = null;
+
final List<String> javaAdditionalArgs = new ArrayList<>();
for (final Map.Entry<String, String> entry : props.entrySet()) {
final String key = entry.getKey();
final String value = entry.getValue();
if (key.startsWith("java.arg")) {
javaAdditionalArgs.add(value);
+ if(value.toLowerCase().startsWith("-xms")) {
Review Comment:
Is there a reason for the `toLowerCase()` comparison? The case should be
consistent.
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java:
##########
@@ -1392,6 +1404,33 @@ public boolean accept(final File dir, final String
filename) {
}
}
+ private String printBasicOSJavaDetails(final String xms, final String xmx)
{
+ Map<String, String> details = new HashMap<String, String>(6);
+
+ // java version
+ details.put("javaVersion", System.getProperty("java.version"));
+
+ // num cores
+ details.put("cores",
Integer.toString(Runtime.getRuntime().availableProcessors()));
+
+ // max file descriptor count + total physical memory
+ try {
+ final ObjectName osObjectName =
ManagementFactory.getOperatingSystemMXBean().getObjectName();
+ final Object maxOpenFileCount =
ManagementFactory.getPlatformMBeanServer().getAttribute(osObjectName,
"MaxFileDescriptorCount");
+ details.put("maxOpenFileDescriptors",
String.valueOf(maxOpenFileCount));
+ final Object totalPhysicalMemory =
ManagementFactory.getPlatformMBeanServer().getAttribute(osObjectName,
"TotalPhysicalMemorySize");
+ details.put("totalPhysicalMemoryMB", String.valueOf(((Long)
totalPhysicalMemory) / (1024*1024)));
+ } catch (final Throwable t) {
+ // Ignore. This will throw either ClassNotFound or
NoClassDefFoundError if unavailable in this JVM.
+ }
+
+ // min/max heap
+ details.put("xms", xms);
+ details.put("xmx", xmx);
Review Comment:
It looks like these values could be `null`, correct? They should be checked
before inserting into the Map.
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java:
##########
@@ -1392,6 +1404,33 @@ public boolean accept(final File dir, final String
filename) {
}
}
+ private String printBasicOSJavaDetails(final String xms, final String xmx)
{
+ Map<String, String> details = new HashMap<String, String>(6);
+
+ // java version
+ details.put("javaVersion", System.getProperty("java.version"));
+
+ // num cores
+ details.put("cores",
Integer.toString(Runtime.getRuntime().availableProcessors()));
Review Comment:
Recommend naming this `availableProcessors` to match the runtime property.
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java:
##########
@@ -1165,13 +1168,22 @@ public void start(final boolean monitor) throws
IOException {
nifiPropsFilename = nifiPropsFilename.trim();
+ String xmx = null;
+ String xms = null;
+
final List<String> javaAdditionalArgs = new ArrayList<>();
for (final Map.Entry<String, String> entry : props.entrySet()) {
final String key = entry.getKey();
final String value = entry.getValue();
if (key.startsWith("java.arg")) {
javaAdditionalArgs.add(value);
+ if(value.toLowerCase().startsWith("-xms")) {
+ xms = StringUtils.substringAfter(value.toLowerCase(),
"-xms");
+ }
+ if(value.toLowerCase().startsWith("-xmx")) {
Review Comment:
```suggestion
if (value.toLowerCase().startsWith("-xmx")) {
```
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java:
##########
@@ -1392,6 +1404,33 @@ public boolean accept(final File dir, final String
filename) {
}
}
+ private String printBasicOSJavaDetails(final String xms, final String xmx)
{
+ Map<String, String> details = new HashMap<String, String>(6);
Review Comment:
Recommend `LinkedHashMap` for guaranteed ordering.
```suggestion
Map<String, String> details = new LinkedHashMap<String, String>(6);
```
##########
nifi-bootstrap/src/main/java/org/apache/nifi/bootstrap/RunNiFi.java:
##########
@@ -1165,13 +1168,22 @@ public void start(final boolean monitor) throws
IOException {
nifiPropsFilename = nifiPropsFilename.trim();
+ String xmx = null;
+ String xms = null;
+
final List<String> javaAdditionalArgs = new ArrayList<>();
for (final Map.Entry<String, String> entry : props.entrySet()) {
final String key = entry.getKey();
final String value = entry.getValue();
if (key.startsWith("java.arg")) {
javaAdditionalArgs.add(value);
+ if(value.toLowerCase().startsWith("-xms")) {
Review Comment:
```suggestion
if (value.toLowerCase().startsWith("-xms")) {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]