Skip to content

Commit 1d2d191

Browse files
authored
Fix memory leak in pinot client BrokerCache (#18290)
1 parent c137b75 commit 1d2d191

2 files changed

Lines changed: 89 additions & 1 deletion

File tree

pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/BrokerCache.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,13 @@ public BrokerCache(Properties properties, String controllerUrl) {
129129
.setConnectTimeout(Duration.ofMillis(connectTimeoutMs))
130130
.setHandshakeTimeout(handshakeTimeoutMs)
131131
.setUserAgent(ConnectionUtils.getUserAgentVersionFromClassPath("ua_broker_cache", appId))
132-
.setEnabledProtocols(tlsProtocols.getEnabledProtocols().toArray(new String[0]));
132+
.setEnabledProtocols(tlsProtocols.getEnabledProtocols().toArray(new String[0]))
133+
// Reuse a JVM-wide Netty I/O thread pool and timer across all BrokerCache instances so
134+
// that the periodic broker refresh does not multiply Netty threads by the number of
135+
// client connections. AHC will not shut these down on close() because they are externally
136+
// supplied (see ChannelManager#allowReleaseEventLoopGroup / AHC#allowStopNettyTimer).
137+
.setEventLoopGroup(PinotClientNettyResources.eventLoopGroup())
138+
.setNettyTimer(PinotClientNettyResources.timer());
133139

134140
_client = Dsl.asyncHttpClient(builder.build());
135141
ControllerRequestURLBuilder controllerRequestURLBuilder =
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.pinot.client;
20+
21+
import io.netty.channel.EventLoopGroup;
22+
import io.netty.channel.nio.NioEventLoopGroup;
23+
import io.netty.util.HashedWheelTimer;
24+
import io.netty.util.Timer;
25+
import io.netty.util.concurrent.DefaultThreadFactory;
26+
27+
28+
/**
29+
* Process-wide shared Netty resources for Pinot client {@code AsyncHttpClient} instances.
30+
*
31+
* <p>Every {@code AsyncHttpClient} built inside the Pinot client (one per {@link BrokerCache},
32+
* per query transport, and per controller transport) owns, by default, its own
33+
* {@link io.netty.channel.EventLoopGroup} and {@link io.netty.util.HashedWheelTimer}. When many
34+
* client connections are kept alive simultaneously (for example behind a JDBC connection pool
35+
* such as HikariCP), this multiplies the number of Netty I/O and timer threads by the number of
36+
* pooled connections. The combination of AHC's default pooled-connection idle timeout (60s) and
37+
* the broker cache refresh interval (5 minutes) additionally causes new TCP connections to be
38+
* opened on every refresh, which lazily spawns fresh {@code NioEventLoop} threads up to each
39+
* group's {@code 2 * availableProcessors()} cap.
40+
*
41+
* <p>AsyncHttpClient natively supports injecting an externally managed {@code EventLoopGroup}
42+
* and {@code Timer} via
43+
* {@link org.asynchttpclient.DefaultAsyncHttpClientConfig.Builder#setEventLoopGroup(EventLoopGroup)}
44+
* and {@link org.asynchttpclient.DefaultAsyncHttpClientConfig.Builder#setNettyTimer(Timer)}; when
45+
* either is supplied externally, AHC will not shut it down on {@code close()} (see
46+
* {@code ChannelManager#allowReleaseEventLoopGroup} and
47+
* {@code DefaultAsyncHttpClient#allowStopNettyTimer}). This class exposes such shared instances
48+
* so that Pinot client AHCs can reuse a single I/O thread pool and a single timer across the
49+
* JVM regardless of how many {@code AsyncHttpClient} instances are created.
50+
*
51+
* <p>The underlying threads are daemon threads; no explicit shutdown is required and they will
52+
* not block JVM exit.
53+
*/
54+
public final class PinotClientNettyResources {
55+
56+
private PinotClientNettyResources() {
57+
}
58+
59+
private static final class Holder {
60+
private static final EventLoopGroup EVENT_LOOP_GROUP =
61+
new NioEventLoopGroup(0, new DefaultThreadFactory("pinot-client-nio", true));
62+
private static final Timer TIMER =
63+
new HashedWheelTimer(new DefaultThreadFactory("pinot-client-timer", true));
64+
}
65+
66+
/**
67+
* Returns the JVM-wide shared {@link EventLoopGroup} for Pinot client {@code AsyncHttpClient}
68+
* instances. The group uses daemon threads and the Netty default size of
69+
* {@code 2 * availableProcessors()}.
70+
*/
71+
public static EventLoopGroup eventLoopGroup() {
72+
return Holder.EVENT_LOOP_GROUP;
73+
}
74+
75+
/**
76+
* Returns the JVM-wide shared {@link Timer} for Pinot client {@code AsyncHttpClient} instances.
77+
* The timer uses a daemon thread.
78+
*/
79+
public static Timer timer() {
80+
return Holder.TIMER;
81+
}
82+
}

0 commit comments

Comments
 (0)