Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: log jemalloc presence at startup, not at shutdown #4738

Merged
merged 5 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

### Additions and Improvements
- Update most dependencies to latest version [#5269](https://github.com/hyperledger/besu/pull/5269)
- If jemalloc is used, print its version in the configuration overview [#4738](https://github.com/hyperledger/besu/pull/4738)

### Bug Fixes

Expand Down
22 changes: 6 additions & 16 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@
import org.hyperledger.besu.util.number.Fraction;
import org.hyperledger.besu.util.number.Percentage;
import org.hyperledger.besu.util.number.PositiveNumber;
import org.hyperledger.besu.util.platform.PlatformDetector;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -1465,8 +1464,6 @@ public int parse(
final int exitCode =
parse(resultHandler, executionExceptionHandler, parameterExceptionHandler, args);

detectJemalloc();

return exitCode;
}

Expand Down Expand Up @@ -1495,6 +1492,7 @@ public void run() {
instantiateSignatureAlgorithmFactory();

logger.info("Starting Besu");

// Need to create vertx after cmdline has been parsed, such that metricsSystem is configurable
vertx = createVertx(createVertxOptions(metricsSystem.get()));

Expand Down Expand Up @@ -1570,18 +1568,6 @@ private void registerConverters() {
commandLine.registerConverter(MetricCategory.class, metricCategoryConverter);
}

private void detectJemalloc() {
// jemalloc is only supported on Linux at the moment
if (PlatformDetector.getOSType().equals("linux")) {
Optional.ofNullable(environment.get("BESU_USING_JEMALLOC"))
.ifPresentOrElse(
present -> logger.info("Using jemalloc"),
() ->
logger.info(
"jemalloc library not found, memory usage may be reduced by installing it"));
}
}

private void handleStableOptions() {
commandLine.addMixin("Ethstats", ethstatsOptions);
commandLine.addMixin("Private key file", nodePrivateKeyFileOption);
Expand Down Expand Up @@ -3622,7 +3608,11 @@ private SyncMode getDefaultSyncModeIfNotSet() {
}

private String generateConfigurationOverview() {
final ConfigurationOverviewBuilder builder = new ConfigurationOverviewBuilder();
final ConfigurationOverviewBuilder builder = new ConfigurationOverviewBuilder(logger);

if (environment != null) {
builder.setEnvironment(environment);
}

if (network != null) {
builder.setNetwork(network.normalize());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,20 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;

import org.slf4j.Logger;
import oshi.PlatformEnum;
import oshi.SystemInfo;
import oshi.hardware.HardwareAbstractionLayer;

/** The Configuration overview builder. */
public class ConfigurationOverviewBuilder {
@SuppressWarnings("PrivateStaticFinalLoggers")
private final Logger logger;

private String network;
private BigInteger networkId;
private boolean hasCustomGenesis;
Expand All @@ -40,6 +47,14 @@ public class ConfigurationOverviewBuilder {
private Collection<String> engineApis;
private String engineJwtFilePath;
private boolean isHighSpec = false;
private Map<String, String> environment;

/**
* @param logger the logger
*/
public ConfigurationOverviewBuilder(final Logger logger) {
this.logger = logger;
}

/**
* Sets network.
Expand Down Expand Up @@ -161,6 +176,17 @@ public ConfigurationOverviewBuilder setEngineJwtFile(final String engineJwtFileP
return this;
}

/**
* Sets the environment variables.
*
* @param environment the enveironment variables
* @return the builder
*/
public ConfigurationOverviewBuilder setEnvironment(final Map<String, String> environment) {
this.environment = environment;
return this;
}

/**
* Build configuration overview.
*
Expand Down Expand Up @@ -226,6 +252,8 @@ public String build() {
if (glibcVersion != null) {
lines.add("glibc: " + glibcVersion);
}

detectJemalloc(lines);
}

final HardwareAbstractionLayer hardwareInfo = new SystemInfo().getHardware();
Expand All @@ -236,6 +264,31 @@ public String build() {
return FramedLogMessage.generate(lines);
}

private void detectJemalloc(final List<String> lines) {
Optional.ofNullable(Objects.isNull(environment) ? null : environment.get("BESU_USING_JEMALLOC"))
.ifPresentOrElse(
t -> {
try {
final String version = PlatformDetector.getJemalloc();
lines.add("jemalloc: " + version);
} catch (final Throwable throwable) {
logger.warn(
"BESU_USING_JEMALLOC is present but we failed to load jemalloc library to get the version",
throwable);
}
},
() -> {
// in case the user is using jemalloc without BESU_USING_JEMALLOC env var
try {
final String version = PlatformDetector.getJemalloc();
lines.add("jemalloc: " + version);
} catch (final Throwable throwable) {
logger.info(
"jemalloc library not found, memory usage may be reduced by installing it");
}
});
}

private String normalizeSize(final long size) {
return String.format("%.02f", (double) (size) / 1024 / 1024 / 1024) + " GB";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5404,11 +5404,15 @@ public void pkiBlockCreationFullConfig() throws Exception {
}

@Test
public void logsUsingJemallocWhenEnvVarPresent() {
public void logsWarningWhenFailToLoadJemalloc() {
assumeThat(PlatformDetector.getOSType(), is("linux"));
setEnvironmentVariable("BESU_USING_JEMALLOC", "true");
parseCommand();
verify(mockLogger).info("Using jemalloc");
verify(mockLogger)
.warn(
eq(
"BESU_USING_JEMALLOC is present but we failed to load jemalloc library to get the version"),
any(Throwable.class));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,22 @@
package org.hyperledger.besu.cli;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Collection;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;

class ConfigurationOverviewBuilderTest {
private ConfigurationOverviewBuilder builder;

@BeforeEach
void setUp() {
builder = new ConfigurationOverviewBuilder();
builder = new ConfigurationOverviewBuilder(mock(Logger.class));
}

@Test
Expand Down
1 change: 1 addition & 0 deletions util/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ dependencies {
api 'org.slf4j:slf4j-api'

implementation 'com.google.guava:guava'
implementation 'net.java.dev.jna:jna'
implementation 'org.apache.commons:commons-lang3'
implementation 'org.apache.logging.log4j:log4j-core'
implementation 'org.apache.logging.log4j:log4j-slf4j2-impl'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import com.sun.jna.Library;
import com.sun.jna.Native;
import com.sun.jna.ptr.IntByReference;
import com.sun.jna.ptr.PointerByReference;

/**
* Detects OS and VMs.
*
Expand All @@ -36,6 +41,7 @@ public class PlatformDetector {
private static String _vm;
private static String _arch;
private static String _glibc;
private static String _jemalloc;

/**
* Gets OS type.
Expand Down Expand Up @@ -98,6 +104,14 @@ public static String getGlibc() {
return _glibc;
}

public static String getJemalloc() {
if (_jemalloc == null) {
detectJemalloc();
}

return _jemalloc;
}

private static final String UNKNOWN = "unknown";

private static void detect() {
Expand Down Expand Up @@ -306,4 +320,23 @@ private static String normalizeGLibcVersion(final String rawGlibcVersion) {

return matcher.find() ? matcher.group() : null;
}

private static void detectJemalloc() {
interface JemallocLib extends Library {
int mallctl(
String property,
PointerByReference value,
IntByReference len,
String newValue,
int newLen);
}

final JemallocLib jemallocLib = Native.load("jemalloc", JemallocLib.class);

PointerByReference pVersion = new PointerByReference();
IntByReference pSize = new IntByReference(Native.POINTER_SIZE);
jemallocLib.mallctl("version", pVersion, pSize, null, 0);

_jemalloc = pVersion.getValue().getString(0);
}
}