Skip to content

Commit 99db9ec

Browse files
authored
Fix UDP socket leak in CentralDogma-to-CentralDogma mirroring (#1328)
Motivation: CentralDogma-to-CentralDogma mirrors were creating a new ArmeriaCentralDogmaClient on every scheduler tick, allocating a fresh Armeria WebClient, connection pool, and UDP socket each time. These sockets were not reclaimed promptly, leading to too many open files. Modifications: - Add CentralDogma.withAccessToken(String) as a default interface method that returns a derived client sharing the same underlying WebClient. Calling close() on the derived client is a no-op; the base client owns the connection resources. - Add Mirror.setBaseClientPool(ConcurrentHashMap<String, Object>) as a default no-op method so MirrorSchedulingService can inject the pool without coupling the server module to client types. Result: - Mirrors pointing at the same remote host share one Armeria WebClient, eliminating per-run DNS/TLS/connection overhead and stopping UDP socket accumulation.
1 parent 93f5356 commit 99db9ec

10 files changed

Lines changed: 131 additions & 25 deletions

File tree

client/java-armeria/src/main/java/com/linecorp/centraldogma/internal/client/armeria/ArmeriaCentralDogma.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
import com.linecorp.armeria.common.util.SafeCloseable;
7878
import com.linecorp.armeria.common.util.TimeoutMode;
7979
import com.linecorp.centraldogma.client.AbstractCentralDogma;
80+
import com.linecorp.centraldogma.client.CentralDogma;
8081
import com.linecorp.centraldogma.client.CentralDogmaRepository;
8182
import com.linecorp.centraldogma.client.RepositoryInfo;
8283
import com.linecorp.centraldogma.common.ApiRequestTimeoutException;
@@ -176,6 +177,14 @@ public ArmeriaCentralDogma(ScheduledExecutorService blockingTaskExecutor,
176177
this.whenReady = whenReady;
177178
}
178179

180+
@Override
181+
public CentralDogma withAccessToken(String accessToken) {
182+
requireNonNull(accessToken, "accessToken");
183+
// Pass a no-op SafeCloseable: the base client owns the connection resources.
184+
return new ArmeriaCentralDogma(executor(), client, accessToken,
185+
() -> {}, meterRegistry(), null);
186+
}
187+
179188
@Override
180189
public CompletableFuture<Void> whenEndpointReady() {
181190
if (whenReady != null) {

client/java/src/main/java/com/linecorp/centraldogma/client/AbstractCentralDogma.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,14 @@ protected final ScheduledExecutorService executor() {
7474
return blockingTaskExecutor;
7575
}
7676

77+
/**
78+
* Returns the {@link MeterRegistry}, or {@code null} if none was configured.
79+
*/
80+
@Nullable
81+
protected final MeterRegistry meterRegistry() {
82+
return meterRegistry;
83+
}
84+
7785
@Override
7886
public CentralDogmaRepository forRepo(String projectName, String repositoryName) {
7987
requireNonNull(projectName, "projectName");

client/java/src/main/java/com/linecorp/centraldogma/client/CentralDogma.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@
5353
*/
5454
public interface CentralDogma extends AutoCloseable {
5555

56+
/**
57+
* Returns a derived {@link CentralDogma} that reuses the underlying connection pool of this client
58+
* but authenticates with the given {@code accessToken}. Calling {@link #close()} on the derived client
59+
* is a no-op; the connection pool is owned by this (base) client.
60+
*
61+
* @throws UnsupportedOperationException if this implementation does not support derived clients
62+
*/
63+
default CentralDogma withAccessToken(String accessToken) {
64+
throw new UnsupportedOperationException(getClass().getName() + " does not support withAccessToken");
65+
}
66+
5667
/**
5768
* Returns a new {@link CentralDogmaRepository} that is used to send a request to the specified
5869
* {@code projectName} and {@code repositoryName}.

client/java/src/main/java/com/linecorp/centraldogma/internal/client/ReplicationLagTolerantCentralDogma.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,14 @@ public ReplicationLagTolerantCentralDogma(ScheduledExecutorService blockingTaskE
106106
this.currentReplicaHintSupplier = currentReplicaHintSupplier;
107107
}
108108

109+
@Override
110+
public CentralDogma withAccessToken(String accessToken) {
111+
requireNonNull(accessToken, "accessToken");
112+
return new ReplicationLagTolerantCentralDogma(
113+
executor(), delegate.withAccessToken(accessToken),
114+
maxRetries, retryIntervalMillis, currentReplicaHintSupplier, meterRegistry());
115+
}
116+
109117
@Override
110118
public CompletableFuture<Void> createProject(String projectName) {
111119
return delegate.createProject(projectName);

it/mirror-listener/src/test/java/com/linecorp/centraldogma/it/mirror/listener/CustomMirrorListenerTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,7 @@ protected MirrorResult mirrorRemoteToLocal(File workDir, CommandExecutor executo
127127
final DefaultMirrorAccessController ac = new DefaultMirrorAccessController();
128128
ac.setRepository(repositoryExtension.crudRepository());
129129
final MirrorSchedulingService service = new MirrorSchedulingService(
130-
temporaryFolder, pm, new SimpleMeterRegistry(), 1, 1, 1, null, false,
131-
ac);
130+
temporaryFolder, pm, new SimpleMeterRegistry(), 1, 1, 1, null, ac);
132131
final CommandExecutor executor = mock(CommandExecutor.class);
133132
service.start(executor);
134133

server-mirror-dogma/src/main/java/com/linecorp/centraldogma/server/internal/mirror/CentralDogmaMirror.java

Lines changed: 63 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.HashMap;
3030
import java.util.List;
3131
import java.util.Map;
32+
import java.util.concurrent.ConcurrentHashMap;
3233

3334
import org.jspecify.annotations.Nullable;
3435
import org.slf4j.Logger;
@@ -52,6 +53,7 @@
5253
import com.linecorp.centraldogma.common.PathPattern;
5354
import com.linecorp.centraldogma.common.RedundantChangeException;
5455
import com.linecorp.centraldogma.common.Revision;
56+
import com.linecorp.centraldogma.internal.CsrfToken;
5557
import com.linecorp.centraldogma.internal.Jackson;
5658
import com.linecorp.centraldogma.internal.Util;
5759
import com.linecorp.centraldogma.server.command.Command;
@@ -72,6 +74,10 @@ public final class CentralDogmaMirror extends AbstractMirror {
7274
private final String remoteProject;
7375
private final String remoteRepo;
7476

77+
// Injected by MirrorSchedulingService before each run; null only in tests / direct usage.
78+
@Nullable
79+
private volatile ConcurrentHashMap<String, Object> baseClientPool;
80+
7581
public CentralDogmaMirror(String id, boolean enabled, Cron schedule, MirrorDirection direction,
7682
Credential credential, Repository localRepo, String localPath,
7783
RepositoryUri remoteUri, String remoteProject, String remoteRepo,
@@ -83,6 +89,11 @@ public CentralDogmaMirror(String id, boolean enabled, Cron schedule, MirrorDirec
8389
this.remoteRepo = requireNonNull(remoteRepo, "remoteRepo");
8490
}
8591

92+
@Override
93+
public void setBaseClientPool(ConcurrentHashMap<String, Object> pool) {
94+
baseClientPool = pool;
95+
}
96+
8697
@VisibleForTesting
8798
String remoteProject() {
8899
return remoteProject;
@@ -93,25 +104,64 @@ String remoteRepo() {
93104
return remoteRepo;
94105
}
95106

96-
private CentralDogma createRemoteClient(long maxNumBytes) throws UnknownHostException {
107+
private static String baseClientPoolKey(URI uri) {
108+
return uri.getHost() + ':' + uri.getPort() + ':' +
109+
SCHEME_DOGMA_HTTPS.equals(uri.getScheme());
110+
}
111+
112+
// maxNumBytes comes from MirrorSchedulingService.maxNumBytesPerMirror, which is fixed at server startup.
113+
private synchronized CentralDogma createRemoteClient(long maxNumBytes) throws UnknownHostException {
97114
final URI uri = remoteUri().uri();
115+
final ConcurrentHashMap<String, Object> pool = baseClientPool;
116+
117+
final Credential cred = credential();
118+
final String token = cred instanceof AccessTokenCredential ?
119+
((AccessTokenCredential) cred).accessToken() : CsrfToken.ANONYMOUS;
120+
121+
CentralDogma base;
122+
if (pool != null) {
123+
// Reuse or create a shared base client (MirrorSchedulingService owns the pool lifecycle).
124+
final String key = baseClientPoolKey(uri);
125+
final Object existing = pool.get(key);
126+
if (existing instanceof CentralDogma) {
127+
base = (CentralDogma) existing;
128+
} else {
129+
base = createClient(uri, maxNumBytes, null);
130+
final Object prev = pool.putIfAbsent(key, base);
131+
if (prev instanceof CentralDogma) {
132+
// Lost the race; close ours and reuse the winner.
133+
try {
134+
base.close();
135+
} catch (Exception e) {
136+
logger.warn("Failed to close the redundant base CentralDogma client: {}", uri, e);
137+
}
138+
base = (CentralDogma) prev;
139+
}
140+
}
141+
142+
// TODO(minwoox) Support mTLS authentication as well.
143+
return base.withAccessToken(token);
144+
}
145+
// No pool injected (tests or direct usage without the scheduler).
146+
return createClient(uri, maxNumBytes, token);
147+
}
148+
149+
private static CentralDogma createClient(URI uri, long maxResponseLength, @Nullable String token)
150+
throws UnknownHostException {
98151
final ArmeriaCentralDogmaBuilder builder = new ArmeriaCentralDogmaBuilder();
99-
final int port = uri.getPort();
100-
if (port > 0) {
101-
builder.host(uri.getHost(), port);
152+
if (uri.getPort() > 0) {
153+
builder.host(uri.getHost(), uri.getPort());
102154
} else {
103155
builder.host(uri.getHost());
104156
}
105-
if (SCHEME_DOGMA_HTTPS.equals(uri.getScheme())) {
106-
builder.useTls(true);
107-
}
108-
final Credential cred = credential();
109-
if (cred instanceof AccessTokenCredential) {
110-
builder.accessToken(((AccessTokenCredential) cred).accessToken());
157+
builder.useTls(SCHEME_DOGMA_HTTPS.equals(uri.getScheme()));
158+
builder.clientConfigurator(cb -> cb.maxResponseLength(maxResponseLength));
159+
// Mirrors run on a fixed schedule; a failed run simply retries on the next tick.
160+
// Persistent health-check traffic is unnecessary and wasteful at scale.
161+
builder.healthCheckIntervalMillis(0);
162+
if (token != null) {
163+
builder.accessToken(token);
111164
}
112-
// TODO(minwoox) Support mTLS authentication as well.
113-
114-
builder.clientConfigurator(cb -> cb.maxResponseLength(maxNumBytes));
115165
return builder.build();
116166
}
117167

server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirrorSchedulingServiceTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ protected MirrorResult mirrorRemoteToLocal(File workDir, CommandExecutor executo
102102
when(mr.mirrors()).thenReturn(CompletableFuture.completedFuture(ImmutableList.of(mirror)));
103103

104104
final MirrorSchedulingService service = new MirrorSchedulingService(
105-
temporaryFolder, pm, new SimpleMeterRegistry(), 1, 1, 1, null, false,
105+
temporaryFolder, pm, new SimpleMeterRegistry(), 1, 1, 1, null,
106106
AlwaysAllowedMirrorAccessController.INSTANCE);
107107
final CommandExecutor executor = mock(CommandExecutor.class);
108108
service.start(executor);

server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/DefaultMirroringServicePlugin.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ public synchronized CompletionStage<Void> start(PluginContext context) {
7777
final int maxNumFilesPerMirror;
7878
final long maxNumBytesPerMirror;
7979
final ZoneConfig zoneConfig;
80-
final boolean runMigration;
8180

8281
if (mirroringServicePluginConfig != null) {
8382
numThreads = mirroringServicePluginConfig.numMirroringThreads();
@@ -93,13 +92,11 @@ public synchronized CompletionStage<Void> start(PluginContext context) {
9392
logger.warn("No 'trustedHostKeys' configured in the mirroring service plugin config. " +
9493
"SSH mirror connections will accept any host key without verification.");
9594
}
96-
runMigration = mirroringServicePluginConfig.runMigration();
9795
} else {
9896
numThreads = MirroringServicePluginConfig.INSTANCE.numMirroringThreads();
9997
maxNumFilesPerMirror = MirroringServicePluginConfig.INSTANCE.maxNumFilesPerMirror();
10098
maxNumBytesPerMirror = MirroringServicePluginConfig.INSTANCE.maxNumBytesPerMirror();
10199
zoneConfig = null;
102-
runMigration = true;
103100
logger.warn("No 'trustedHostKeys' configured in the mirroring service plugin config. " +
104101
"SSH mirror connections will accept any host key without verification.");
105102
}
@@ -108,7 +105,7 @@ public synchronized CompletionStage<Void> start(PluginContext context) {
108105
context.meterRegistry(),
109106
numThreads,
110107
maxNumFilesPerMirror,
111-
maxNumBytesPerMirror, zoneConfig, runMigration,
108+
maxNumBytesPerMirror, zoneConfig,
112109
context.mirrorAccessController());
113110
this.mirroringService = mirroringService;
114111
}

server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/MirrorSchedulingService.java

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.List;
2828
import java.util.ServiceLoader;
2929
import java.util.concurrent.CompletableFuture;
30+
import java.util.concurrent.ConcurrentHashMap;
3031
import java.util.concurrent.ExecutionException;
3132
import java.util.concurrent.ExecutorService;
3233
import java.util.concurrent.Executors;
@@ -104,27 +105,28 @@ public static MirrorListener mirrorListener() {
104105
private final String currentZone;
105106
private final MirrorAccessController mirrorAccessController;
106107
private final AtomicInteger numActiveMirrors = new AtomicInteger();
108+
// Shared base-client pool for CentralDogmaMirror instances. Keyed by "host:port:tls:maxBytes".
109+
// Owned here so stop() can close all entries; injected into each mirror before it runs.
110+
private final ConcurrentHashMap<String, Object> baseClientPool = new ConcurrentHashMap<>();
107111

108112
private volatile CommandExecutor commandExecutor;
109113
private volatile ListeningScheduledExecutorService scheduler;
110114
private volatile ListeningExecutorService worker;
111115
private volatile boolean closing;
112116

117+
@Nullable
113118
private ZonedDateTime lastExecutionTime;
114119
private final MeterRegistry meterRegistry;
115-
// Used to disable in the tests.
116-
private final boolean runMigration;
117120

118121
@VisibleForTesting
119122
public MirrorSchedulingService(File workDir, ProjectManager projectManager, MeterRegistry meterRegistry,
120123
int numThreads, int maxNumFilesPerMirror, long maxNumBytesPerMirror,
121-
@Nullable ZoneConfig zoneConfig, boolean runMigration,
124+
@Nullable ZoneConfig zoneConfig,
122125
MirrorAccessController mirrorAccessController) {
123126

124127
this.workDir = requireNonNull(workDir, "workDir");
125128
this.projectManager = requireNonNull(projectManager, "projectManager");
126129
this.meterRegistry = requireNonNull(meterRegistry, "meterRegistry");
127-
this.runMigration = runMigration;
128130

129131
checkArgument(numThreads > 0, "numThreads: %s (expected: > 0)", numThreads);
130132
checkArgument(maxNumFilesPerMirror > 0,
@@ -227,6 +229,16 @@ public synchronized void stop() {
227229
} finally {
228230
this.scheduler = null;
229231
this.worker = null;
232+
baseClientPool.forEach((key, client) -> {
233+
if (client instanceof AutoCloseable) {
234+
try {
235+
((AutoCloseable) client).close();
236+
} catch (Exception e) {
237+
logger.warn("Failed to close base CentralDogma client for key: {}", key, e);
238+
}
239+
}
240+
});
241+
baseClientPool.clear();
230242
}
231243
}
232244

@@ -310,6 +322,7 @@ private void scheduleMirrors() {
310322
}
311323
try {
312324
if (m.nextExecutionTime(currentLastExecutionTime).compareTo(now) < 0) {
325+
m.setBaseClientPool(baseClientPool);
313326
runAsync(new MirrorTask(m, User.SYSTEM, Instant.now(),
314327
currentZone, true));
315328
}
@@ -334,7 +347,10 @@ public CompletableFuture<Void> mirror() {
334347
}
335348
try {
336349
p.metaRepo().mirrors().get(5, TimeUnit.SECONDS)
337-
.forEach(m -> run(new MirrorTask(m, User.SYSTEM, Instant.now(), currentZone, false)));
350+
.forEach(m -> {
351+
m.setBaseClientPool(baseClientPool);
352+
run(new MirrorTask(m, User.SYSTEM, Instant.now(), currentZone, false));
353+
});
338354
} catch (InterruptedException | TimeoutException | ExecutionException e) {
339355
throw new IllegalStateException(
340356
"Failed to load mirror list with in 5 seconds. project: " + p.name(), e);

server/src/main/java/com/linecorp/centraldogma/server/mirror/Mirror.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.net.URI;
2020
import java.time.Instant;
2121
import java.time.ZonedDateTime;
22+
import java.util.concurrent.ConcurrentHashMap;
2223

2324
import org.jspecify.annotations.Nullable;
2425

@@ -131,4 +132,11 @@ default String remoteBranch() {
131132
*/
132133
MirrorResult mirror(File workDir, CommandExecutor executor, int maxNumFiles, long maxNumBytes,
133134
Instant triggeredTime);
135+
136+
/**
137+
* Injects the shared base-client pool managed by the mirroring scheduler. The pool maps a string key
138+
* (encoding host, port, TLS flag, and maxResponseLength) to a shared base client instance. Only
139+
* CentralDogma-type mirrors use this; the default implementation is a no-op.
140+
*/
141+
default void setBaseClientPool(ConcurrentHashMap<String, Object> baseClientPool) {}
134142
}

0 commit comments

Comments
 (0)