Skip to content

Commit

Permalink
Replace supplier of ParameterizedMessage with java.util.Supplier<Stri…
Browse files Browse the repository at this point in the history
…ng> (#86971)

This is a result of structural search/replace in intellij. This only affects log methods with a signature
logger.info(Supplier<?>) where level could be info/debug etc and supplier argument is in a form of
()-> new ParameterizedMessage

This commit also introduced a Strings utility class to avoid passing Locale.ROOT to every
String.format(Locale.ROOT, pattern, args)
relates #86549
  • Loading branch information
pgomulka committed May 23, 2022
1 parent 005a2ff commit 24fa003
Show file tree
Hide file tree
Showing 88 changed files with 412 additions and 468 deletions.
28 changes: 28 additions & 0 deletions libs/core/src/main/java/org/elasticsearch/core/Strings.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.core;

import java.util.Locale;

/**
* Utilities related to String class
*/
public class Strings {

/**
* Returns a formatted string using the specified format string and
* arguments.
*
* This method calls {@link String#format(Locale, String, Object...)}
* with Locale.ROOT
*/
public static String format(String format, Object... args) {
return String.format(Locale.ROOT, format, args);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.cluster.metadata.RepositoryMetadata;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.blobstore.BlobContainer;
Expand Down Expand Up @@ -81,6 +80,8 @@
import java.util.function.BiPredicate;
import java.util.stream.StreamSupport;

import static org.elasticsearch.core.Strings.format;

public class AzureBlobStore implements BlobStore {
private static final Logger logger = LogManager.getLogger(AzureBlobStore.class);
private static final long DEFAULT_READ_CHUNK_SIZE = new ByteSizeValue(32, ByteSizeUnit.MB).getBytes();
Expand Down Expand Up @@ -286,7 +287,7 @@ private static Mono<Void> getDeleteTask(String blobName, BlobAsyncClient blobAsy
}

public InputStream getInputStream(String blob, long position, final @Nullable Long length) throws IOException {
logger.trace(() -> new ParameterizedMessage("reading container [{}], blob [{}]", container, blob));
logger.trace(() -> format("reading container [%s], blob [%s]", container, blob));
final AzureBlobServiceClient azureBlobServiceClient = getAzureBlobServiceClientClient();
final BlobServiceClient syncClient = azureBlobServiceClient.getSyncClient();
final BlobServiceAsyncClient asyncClient = azureBlobServiceClient.getAsyncClient();
Expand Down Expand Up @@ -315,7 +316,7 @@ public InputStream getInputStream(String blob, long position, final @Nullable Lo

public Map<String, BlobMetadata> listBlobsByPrefix(String keyPath, String prefix) throws IOException {
final var blobsBuilder = new HashMap<String, BlobMetadata>();
logger.trace(() -> new ParameterizedMessage("listing container [{}], keyPath [{}], prefix [{}]", container, keyPath, prefix));
logger.trace(() -> format("listing container [%s], keyPath [%s], prefix [%s]", container, keyPath, prefix));
try {
final BlobServiceClient client = client();
SocketAccess.doPrivilegedVoidException(() -> {
Expand Down Expand Up @@ -426,7 +427,7 @@ protected void onFailure() {
public void writeBlob(String blobName, InputStream inputStream, long blobSize, boolean failIfAlreadyExists) throws IOException {
assert inputStream.markSupported()
: "Should not be used with non-mark supporting streams as their retry handling in the SDK is broken";
logger.trace(() -> new ParameterizedMessage("writeBlob({}, stream, {})", blobName, blobSize));
logger.trace(() -> format("writeBlob(%s, stream, %s)", blobName, blobSize));
try {
if (blobSize <= getLargeBlobThresholdInBytes()) {
final Flux<ByteBuffer> byteBufferFlux = convertStreamToByteBuffer(inputStream, blobSize, DEFAULT_UPLOAD_BUFFERS_SIZE);
Expand All @@ -445,7 +446,7 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize, b
throw new IOException("Unable to write blob " + blobName, e);
}

logger.trace(() -> new ParameterizedMessage("writeBlob({}, stream, {}) - done", blobName, blobSize));
logger.trace(() -> format("writeBlob(%s, stream, %s) - done", blobName, blobSize));
}

private void executeSingleUpload(String blobName, Flux<ByteBuffer> byteBufferFlux, long blobSize, boolean failIfAlreadyExists) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.cluster.metadata.RepositoryMetadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
Expand All @@ -29,6 +28,7 @@
import java.util.Map;
import java.util.function.Function;

import static org.elasticsearch.core.Strings.format;
import static org.elasticsearch.repositories.azure.AzureStorageService.MAX_CHUNK_SIZE;
import static org.elasticsearch.repositories.azure.AzureStorageService.MIN_CHUNK_SIZE;

Expand Down Expand Up @@ -155,8 +155,8 @@ protected AzureBlobStore createBlobStore() {
final AzureBlobStore blobStore = new AzureBlobStore(metadata, storageService, bigArrays);

logger.debug(
() -> new ParameterizedMessage(
"using container [{}], chunk_size [{}], compress [{}], base_path [{}]",
() -> format(
"using container [%s], chunk_size [%s], compress [%s], base_path [%s]",
blobStore,
chunkSize,
isCompress(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.util.Maps;
Expand All @@ -42,6 +41,7 @@

import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Collections.emptyMap;
import static org.elasticsearch.core.Strings.format;

public class GoogleCloudStorageService {

Expand Down Expand Up @@ -107,9 +107,7 @@ public Storage client(final String clientName, final String repositoryName, fina
);
}

logger.debug(
() -> new ParameterizedMessage("creating GCS client with client_name [{}], endpoint [{}]", clientName, settings.getHost())
);
logger.debug(() -> format("creating GCS client with client_name [%s], endpoint [%s]", clientName, settings.getHost()));
final Storage storage = createClient(settings, stats);
clientCache = Maps.copyMapWithAddedEntry(clientCache, repositoryName, storage);
return storage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
import java.util.Set;
import java.util.function.Predicate;

import static org.elasticsearch.core.Strings.format;

public class ShardStateAction {

private static final Logger logger = LogManager.getLogger(ShardStateAction.class);
Expand Down Expand Up @@ -882,7 +884,7 @@ public void onFailure(Exception e) {
@Override
public void onFailure(Exception e) {
if (e instanceof NotMasterException) {
logger.debug(() -> new ParameterizedMessage("{} no longer master while starting shard [{}]", entry.shardId, entry));
logger.debug(() -> format("%s no longer master while starting shard [%s]", entry.shardId, entry));
} else if (e instanceof FailedToCommitClusterStateException) {
logger.debug(() -> new ParameterizedMessage("{} unexpected failure while starting shard [{}]", entry.shardId, entry), e);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
import java.util.stream.StreamSupport;

import static org.elasticsearch.cluster.coordination.NoMasterBlockService.NO_MASTER_BLOCK_ID;
import static org.elasticsearch.core.Strings.format;
import static org.elasticsearch.gateway.ClusterStateUpdaters.hideStateIfNotRecovered;
import static org.elasticsearch.gateway.GatewayService.STATE_NOT_RECOVERED_BLOCK;
import static org.elasticsearch.monitor.StatusInfo.Status.UNHEALTHY;
Expand Down Expand Up @@ -1332,8 +1333,8 @@ public void publish(
synchronized (mutex) {
if (mode != Mode.LEADER || getCurrentTerm() != clusterStatePublicationEvent.getNewState().term()) {
logger.debug(
() -> new ParameterizedMessage(
"[{}] failed publication as node is no longer master for term {}",
() -> format(
"[%s] failed publication as node is no longer master for term %s",
clusterStatePublicationEvent.getSummary(),
clusterStatePublicationEvent.getNewState().term()
)
Expand All @@ -1351,8 +1352,8 @@ public void publish(
if (currentPublication.isPresent()) {
assert false : "[" + currentPublication.get() + "] in progress, cannot start new publication";
logger.warn(
() -> new ParameterizedMessage(
"[{}] failed publication as already publication in progress",
() -> format(
"[%s] failed publication as already publication in progress",
clusterStatePublicationEvent.getSummary()
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;

import static org.elasticsearch.core.Strings.format;

/**
* Implements the low-level mechanics of sending a cluster state to other nodes in the cluster during a publication.
* <p>
Expand Down Expand Up @@ -395,8 +397,8 @@ private void sendClusterStateDiff(DiscoveryNode destination, ActionListener<Publ
if (e instanceof final TransportException transportException) {
if (transportException.unwrapCause() instanceof IncompatibleClusterStateVersionException) {
logger.debug(
() -> new ParameterizedMessage(
"resending full cluster state to node {} reason {}",
() -> format(
"resending full cluster state to node %s reason %s",
destination,
transportException.getDetailedMessage()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
Expand Down Expand Up @@ -85,6 +84,9 @@
import java.util.function.Consumer;
import java.util.stream.Collectors;

import static java.util.stream.Collectors.joining;
import static org.elasticsearch.core.Strings.format;

/**
* Service responsible for submitting open/close index requests as well as for adding index blocks
*/
Expand Down Expand Up @@ -363,12 +365,7 @@ static ClusterState addIndexClosedBlocks(
blockedIndices.put(index, indexBlock);
}

logger.info(
() -> new ParameterizedMessage(
"closing indices {}",
blockedIndices.keySet().stream().map(Object::toString).collect(Collectors.joining(","))
)
);
logger.info(() -> format("closing indices %s", blockedIndices.keySet().stream().map(Object::toString).collect(joining(","))));
return ClusterState.builder(currentState).blocks(blocks).build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
package org.elasticsearch.common.breaker;

import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.indices.breaker.BreakerSettings;
import org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService;

import java.util.concurrent.atomic.AtomicLong;

import static org.elasticsearch.core.Strings.format;

/**
* Breaker that will check a parent's when incrementing
*/
Expand Down Expand Up @@ -45,7 +46,7 @@ public ChildMemoryCircuitBreaker(BreakerSettings settings, Logger logger, Hierar
this.used = new AtomicLong(0);
this.trippedCount = new AtomicLong(0);
this.logger = logger;
logger.trace(() -> new ParameterizedMessage("creating ChildCircuitBreaker with settings {}", settings));
logger.trace(() -> format("creating ChildCircuitBreaker with settings %s", settings));
this.parent = parent;
}

Expand All @@ -72,7 +73,7 @@ public void circuitBreak(String fieldName, long bytesNeeded) {
+ "/"
+ new ByteSizeValue(memoryBytesLimit)
+ "]";
logger.debug(() -> new ParameterizedMessage("{}", message));
logger.debug(() -> format("%s", message));
throw new CircuitBreakingException(message, bytesNeeded, memoryBytesLimit, durability);
}

Expand Down Expand Up @@ -120,8 +121,8 @@ private long noLimit(long bytes, String label) {
long newUsed;
newUsed = this.used.addAndGet(bytes);
logger.trace(
() -> new ParameterizedMessage(
"[{}] Adding [{}][{}] to used bytes [new used: [{}], limit: [-1b]]",
() -> format(
"[%s] Adding [%s][%s] to used bytes [new used: [%s], limit: [-1b]]",
this.name,
new ByteSizeValue(bytes),
label,
Expand Down Expand Up @@ -183,7 +184,7 @@ private long limit(long bytes, String label, double overheadConstant, long memor
@Override
public void addWithoutBreaking(long bytes) {
long u = used.addAndGet(bytes);
logger.trace(() -> new ParameterizedMessage("[{}] Adjusted breaker by [{}] bytes, now [{}]", this.name, bytes, u));
logger.trace(() -> format("[%s] Adjusted breaker by [%s] bytes, now [%s]", this.name, bytes, u));
assert u >= 0 : "Used bytes: [" + u + "] must be >= 0";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.core.Releasables;
import org.elasticsearch.gateway.AsyncShardFetch.Lister;
import org.elasticsearch.gateway.TransportNodesListGatewayStartedShards.NodeGatewayStartedShards;
Expand All @@ -42,6 +41,9 @@
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Collectors;

import static org.elasticsearch.common.util.set.Sets.difference;
import static org.elasticsearch.core.Strings.format;

public class GatewayAllocator implements ExistingShardsAllocator {

public static final String ALLOCATOR_NAME = "gateway_allocator";
Expand Down Expand Up @@ -181,9 +183,9 @@ private void ensureAsyncFetchStorePrimaryRecency(RoutingAllocation allocation) {
// ways we could decide to cancel a recovery based on stale data (e.g. changing allocation filters or a primary failure) but
// making the wrong decision here is not catastrophic so we only need to cover the common case.
logger.trace(
() -> new ParameterizedMessage(
"new nodes {} found, clearing primary async-fetch-store cache",
Sets.difference(newEphemeralIds, lastSeenEphemeralIds)
() -> format(
"new nodes %s found, clearing primary async-fetch-store cache",
difference(newEphemeralIds, lastSeenEphemeralIds)
)
);
asyncFetchStore.values().forEach(fetch -> clearCacheForPrimary(fetch, allocation));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;

import static org.elasticsearch.core.Strings.format;
import static org.elasticsearch.http.HttpTransportSettings.SETTING_HTTP_BIND_HOST;
import static org.elasticsearch.http.HttpTransportSettings.SETTING_HTTP_MAX_CONTENT_LENGTH;
import static org.elasticsearch.http.HttpTransportSettings.SETTING_HTTP_PORT;
Expand Down Expand Up @@ -341,7 +342,7 @@ protected void serverAcceptedChannel(HttpChannel httpChannel) {
}));
totalChannelsAccepted.incrementAndGet();
httpClientStatsTracker.addClientStats(httpChannel);
logger.trace(() -> new ParameterizedMessage("Http channel accepted: {}", httpChannel));
logger.trace(() -> format("Http channel accepted: %s", httpChannel));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.elasticsearch.index.engine;

import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.lucene.document.LongPoint;
import org.apache.lucene.document.NumericDocValuesField;
import org.apache.lucene.index.DirectoryReader;
Expand Down Expand Up @@ -109,6 +108,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.elasticsearch.core.Strings.format;

public class InternalEngine extends Engine {

/**
Expand Down Expand Up @@ -501,8 +502,8 @@ private void recoverFromTranslogInternal(TranslogRecoveryRunner translogRecovery
assert pendingTranslogRecovery.get() : "translogRecovery is not pending but should be";
pendingTranslogRecovery.set(false); // we are good - now we can commit
logger.trace(
() -> new ParameterizedMessage(
"flushing post recovery from translog: ops recovered [{}], current translog generation [{}]",
() -> format(
"flushing post recovery from translog: ops recovered [%s], current translog generation [%s]",
opsRecovered,
translog.currentFileGeneration()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.elasticsearch.common.breaker.ChildMemoryCircuitBreaker;
import org.elasticsearch.common.breaker.CircuitBreaker;
import org.elasticsearch.common.breaker.CircuitBreakingException;
Expand Down Expand Up @@ -40,6 +39,7 @@
import java.util.function.LongSupplier;
import java.util.stream.Collectors;

import static org.elasticsearch.core.Strings.format;
import static org.elasticsearch.indices.breaker.BreakerSettings.CIRCUIT_BREAKER_LIMIT_SETTING;
import static org.elasticsearch.indices.breaker.BreakerSettings.CIRCUIT_BREAKER_OVERHEAD_SETTING;

Expand Down Expand Up @@ -211,7 +211,7 @@ public HierarchyCircuitBreakerService(Settings settings, List<BreakerSettings> c
CircuitBreaker.Type.PARENT,
null
);
logger.trace(() -> new ParameterizedMessage("parent circuit breaker with settings {}", this.parentSettings));
logger.trace(() -> format("parent circuit breaker with settings %s", this.parentSettings));

this.trackRealMemoryUsage = USE_REAL_MEMORY_USAGE_SETTING.get(settings);

Expand Down Expand Up @@ -436,7 +436,7 @@ public void checkParentLimit(long newBytesReserved, String label) throws Circuit
CircuitBreaker.Durability durability = memoryUsed.transientChildUsage >= memoryUsed.permanentChildUsage
? CircuitBreaker.Durability.TRANSIENT
: CircuitBreaker.Durability.PERMANENT;
logger.debug(() -> new ParameterizedMessage("{}", message.toString()));
logger.debug(() -> format("%s", message.toString()));
throw new CircuitBreakingException(message.toString(), memoryUsed.totalUsage, parentLimit, durability);
}
}
Expand Down

0 comments on commit 24fa003

Please sign in to comment.