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.

Sign up for GitHub

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 a few toString implementations+usages that affect test performance #112380

Merged
merged 4 commits into from
Aug 30, 2024
Merged

Fix a few toString implementations+usages that affect test performance #112380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3a325b8
Fix a few toString implementations+usages that affect test performance
original-brownbear Aug 29, 2024
62a6d4b
Merge remote-tracking branch 'elastic/main' into fix-toString-for-tests
original-brownbear Aug 29, 2024
5fe5669
fix test
original-brownbear Aug 29, 2024
f5a224f
Merge remote-tracking branch 'elastic/main' into fix-toString-for-tests
original-brownbear Aug 30, 2024
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
5 changes: 3 additions & 2 deletions libs/core/src/main/java/org/elasticsearch/core/Releasables.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,9 @@ public static Releasable assertOnce(final Releasable delegate) {
private final AtomicReference<Exception> firstCompletion = new AtomicReference<>();

private void assertFirstRun() {
var previousRun = firstCompletion.compareAndExchange(null, new Exception(delegate.toString()));
assert previousRun == null : previousRun; // reports the stack traces of both completions
var previousRun = firstCompletion.compareAndExchange(null, new Exception("already executed"));
// reports the stack traces of both completions
assert previousRun == null : new AssertionError(delegate.toString(), previousRun);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion libs/core/src/test/java/org/elasticsearch/core/ReleasablesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public String toString() {
.anyMatch(ste -> ste.toString().contains("CloserWithIdentifiableMethodNames.closeMethod2"))
);
assertTrue(
Arrays.stream(assertionError.getCause().getStackTrace())
Arrays.stream(assertionError.getCause().getCause().getStackTrace())
.anyMatch(ste -> ste.toString().contains("CloserWithIdentifiableMethodNames.closeMethod1"))
);
}
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/elasticsearch/action/ActionListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,8 @@ static <Response> ActionListener<Response> assertOnce(ActionListener<Response> d
private final AtomicReference<ElasticsearchException> firstCompletion = new AtomicReference<>();

private void assertFirstRun() {
var previousRun = firstCompletion.compareAndExchange(null, new ElasticsearchException(delegate.toString()));
assert previousRun == null : previousRun; // reports the stack traces of both completions
var previousRun = firstCompletion.compareAndExchange(null, new ElasticsearchException("executed already"));
assert previousRun == null : "[" + delegate + "] " + previousRun; // reports the stack traces of both completions
}

@Override
Expand Down
7 changes: 4 additions & 3 deletions server/src/main/java/org/elasticsearch/common/network/InetAddresses.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.elasticsearch.common.network;

import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.core.Tuple;

import java.net.Inet4Address;
Expand Down Expand Up @@ -246,14 +247,14 @@ public static String toUriString(InetAddress ip) {
* @return {@code String} containing the text-formatted IP address
* @since 10.0
*/
@SuppressForbidden(reason = "java.net.Inet4Address#getHostAddress() is fine no need to duplicate its code")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment for this suggests "use NetworkAddress format() to print IP or IP+ports". Is that a better way to achieve this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, I think the problem is only with the ipv6 version of this method, but it seems impossible to selectively catch that only with forbidden apis?

public static String toAddrString(InetAddress ip) {
if (ip == null) {
throw new NullPointerException("ip");
}
if (ip instanceof Inet4Address) {
if (ip instanceof Inet4Address inet4Address) {
// For IPv4, Java's formatting is good enough.
byte[] bytes = ip.getAddress();
return (bytes[0] & 0xff) + "." + (bytes[1] & 0xff) + "." + (bytes[2] & 0xff) + "." + (bytes[3] & 0xff);
return inet4Address.getHostAddress();
}
if ((ip instanceof Inet6Address) == false) {
throw new IllegalArgumentException("ip");
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/elasticsearch/common/network/NetworkAddress.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ private NetworkAddress() {}
* @return formatted string
*/
public static String format(InetAddress address) {
return format(address, new PortsRange(""));
return InetAddresses.toAddrString(address);
}

/**
Expand Down Expand Up @@ -96,7 +96,7 @@ public static String format(InetSocketAddress address) {
* @return formatted string
*/
public static String format(InetAddress address, int port) {
return format(address, new PortsRange(String.valueOf(port)));
return (address instanceof Inet6Address ? InetAddresses.toUriString(address) : InetAddresses.toAddrString(address)) + ":" + port;
}

/**
Expand Down
3 changes: 1 addition & 2 deletions server/src/main/java/org/elasticsearch/transport/TaskTransportChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.elasticsearch.transport;

import org.elasticsearch.TransportVersion;
import org.elasticsearch.common.Strings;
import org.elasticsearch.core.Releasable;

public class TaskTransportChannel implements TransportChannel {
Expand Down Expand Up @@ -58,6 +57,6 @@ public TransportChannel getChannel() {

@Override
public String toString() {
return Strings.format("TaskTransportChannel{task=%d}{%s}", taskId, channel);
return "TaskTransportChannel{task=" + taskId + "}{" + channel + "}";
}
}
3 changes: 1 addition & 2 deletions server/src/main/java/org/elasticsearch/transport/TcpTransportChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.elasticsearch.transport;

import org.elasticsearch.TransportVersion;
import org.elasticsearch.common.Strings;
import org.elasticsearch.core.Releasable;

public final class TcpTransportChannel implements TransportChannel {
Expand Down Expand Up @@ -89,6 +88,6 @@ public TcpChannel getChannel() {

@Override
public String toString() {
return Strings.format("TcpTransportChannel{req=%d}{%s}{%s}", requestId, action, channel);
return "TcpTransportChannel{req=" + requestId + "}{" + action + "}{" + channel + "}";
}
}