Skip to content

Commit

Permalink
Exclude attributes from Endpoint.toString() (#6061)
Browse files Browse the repository at this point in the history
Motivation:

We observed `OutOfMemoryError` in internal CI tests when a new
`Endpoint` is created.
```
com.linecorp.armeria.xds.client.endpoint.EndpointsPool$$Lambda$754/1708334230@3a58f44e
    java.lang.OutOfMemoryError: Java heap space
        at java.util.Arrays.copyOfRange(Arrays.java:3664)
        at java.lang.String.<init>(String.java:207)
        at java.lang.StringBuilder.toString(StringBuilder.java:412)
        at com.google.protobuf.TextFormat$Printer.printToString(TextFormat.java:593)
        at com.google.protobuf.AbstractMessage.toString(AbstractMessage.java:87)
        at java.lang.String.valueOf(String.java:2994)
        at java.lang.StringBuilder.append(StringBuilder.java:136)
        at com.linecorp.armeria.internal.shaded.guava.collect.AbstractMapEntry.toString(AbstractMapEntry.java:70)
        at java.lang.String.valueOf(String.java:2994)
        at java.lang.StringBuilder.append(StringBuilder.java:136)
        at com.linecorp.armeria.internal.shaded.guava.collect.Iterators.toString(Iterators.java:294)
        at com.linecorp.armeria.common.ImmutableAttributes.toString(ImmutableAttributes.java:183)
        at java.lang.String.valueOf(String.java:2994)
        at java.lang.StringBuilder.append(StringBuilder.java:136)
        at com.linecorp.armeria.client.Endpoint.generateToString(Endpoint.java:304)
        at com.linecorp.armeria.client.Endpoint.<init>(Endpoint.java:268)
        at com.linecorp.armeria.client.Endpoint.replaceAttrs(Endpoint.java:747)
        at com.linecorp.armeria.client.Endpoint.withAttr(Endpoint.java:707)
        at com.linecorp.armeria.xds.client.endpoint.EndpointsPool.withTimestamp(EndpointsPool.java:103)
        at com.linecorp.armeria.xds.client.endpoint.EndpointsPool.cacheAttributesAndDelegate(EndpointsPool.java:71)
        at com.linecorp.armeria.xds.client.endpoint.EndpointsPool.lambda$updateClusterSnapshot$1(EndpointsPool.java:62)
        at com.linecorp.armeria.xds.client.endpoint.EndpointsPool$$Lambda$754/1708334230.run(Unknown Source)
```
When an `Endpoint` is initicated, it pre-generates a string
representation for caching and reuse. In our tests, it contained many
attributes related to xDS for service discovery. It might be a good idea
to remove attributes whose size is hard to predict from `toString()` and
prevent unintended OOM.

In addition, the result size of `toString()` gets small, so I doubt that
the pre-generated cache is useful for performance.

Modifications:

- Remove attributes from `toString()`.
- Lazily build the string representation when `toString()` is called and
don't cache the result.

Result:

`Endpoint` no longer includes attributes in `.toString()`.
  • Loading branch information
ikhoon authored Jan 14, 2025
1 parent 8ae035e commit 6562927
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 21 deletions.
29 changes: 9 additions & 20 deletions core/src/main/java/com/linecorp/armeria/client/Endpoint.java
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ enum Type {
private final int weight;
private final List<Endpoint> endpoints;
private final String authority;
private final String strVal;

@Nullable
private final Attributes attributes;
Expand Down Expand Up @@ -264,8 +263,6 @@ private Endpoint(Type type, String host, @Nullable String ipAddr, int port, int

// Pre-generate the authority.
authority = generateAuthority(type, host, port);
// Pre-generate toString() value.
strVal = generateToString(type, authority, ipAddr, weight, attributes);
this.attributes = attributes;
}

Expand All @@ -291,22 +288,6 @@ private static String generateAuthority(Type type, String host, int port) {
}
}

private static String generateToString(Type type, String authority, @Nullable String ipAddr,
int weight, @Nullable Attributes attributes) {
try (TemporaryThreadLocals tempThreadLocals = TemporaryThreadLocals.acquire()) {
final StringBuilder buf = tempThreadLocals.stringBuilder();
buf.append("Endpoint{").append(authority);
if (type == Type.HOSTNAME_AND_IP) {
buf.append(", ipAddr=").append(ipAddr);
}
buf.append(", weight=").append(weight);
if (attributes != null) {
buf.append(", attributes=").append(attributes);
}
return buf.append('}').toString();
}
}

@Override
public List<Endpoint> endpoints() {
return endpoints;
Expand Down Expand Up @@ -981,6 +962,14 @@ public int compareTo(Endpoint that) {

@Override
public String toString() {
return strVal;
try (TemporaryThreadLocals tempThreadLocals = TemporaryThreadLocals.acquire()) {
final StringBuilder buf = tempThreadLocals.stringBuilder();
buf.append("Endpoint{").append(authority);
if (type == Type.HOSTNAME_AND_IP) {
buf.append(", ipAddr=").append(ipAddr);
}
return buf.append(", weight=").append(weight)
.append('}').toString();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -539,8 +539,9 @@ void testToString() {

// attributes
final Endpoint endpointWithAttr = Endpoint.of("127.0.0.1").withAttr(AttributeKey.valueOf("test"), 1);
// toString() should not include the attributes.
assertThat(endpointWithAttr.toString())
.isEqualTo("Endpoint{127.0.0.1, weight=1000, attributes=[test=1]}");
.isEqualTo("Endpoint{127.0.0.1, weight=1000}");
}

@Test
Expand Down

0 comments on commit 6562927

Please sign in to comment.