Skip to content

Commit e16745b

Browse files
committed
xds: Use ObjectPool to manage scheduler in XdsServerWrapper
Also, we should be passing the executor in all tests, as we don't want other threads to be running if we can avoid it.
1 parent 1e85674 commit e16745b

4 files changed

Lines changed: 30 additions & 24 deletions

File tree

‎xds/src/main/java/io/grpc/xds/XdsServerBuilder.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
import io.grpc.Server;
3232
import io.grpc.ServerBuilder;
3333
import io.grpc.ServerCredentials;
34+
import io.grpc.internal.GrpcUtil;
35+
import io.grpc.internal.SharedResourcePool;
3436
import io.grpc.netty.InternalNettyServerBuilder;
3537
import io.grpc.netty.InternalNettyServerCredentials;
3638
import io.grpc.netty.InternalProtocolNegotiator;
@@ -128,7 +130,8 @@ public Server build() {
128130
}
129131
InternalNettyServerBuilder.eagAttributes(delegate, builder.build());
130132
return new XdsServerWrapper("0.0.0.0:" + port, delegate, xdsServingStatusListener,
131-
filterChainSelectorManager, xdsClientPoolFactory, bootstrapOverride, filterRegistry);
133+
filterChainSelectorManager, xdsClientPoolFactory, bootstrapOverride, filterRegistry,
134+
SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE));
132135
}
133136

134137
@VisibleForTesting

‎xds/src/main/java/io/grpc/xds/XdsServerWrapper.java‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,8 @@
4545
import io.grpc.StatusOr;
4646
import io.grpc.SynchronizationContext;
4747
import io.grpc.SynchronizationContext.ScheduledHandle;
48-
import io.grpc.internal.GrpcUtil;
48+
import io.grpc.internal.FixedObjectPool;
4949
import io.grpc.internal.ObjectPool;
50-
import io.grpc.internal.SharedResourceHolder;
5150
import io.grpc.xds.EnvoyServerProtoData.FilterChain;
5251
import io.grpc.xds.Filter.FilterConfig;
5352
import io.grpc.xds.Filter.NamedFilterConfig;
@@ -100,7 +99,7 @@ public void uncaughtException(Thread t, Throwable e) {
10099
static final long RETRY_DELAY_NANOS = TimeUnit.MINUTES.toNanos(1);
101100
private final String listenerAddress;
102101
private final ServerBuilder<?> delegateBuilder;
103-
private boolean sharedTimeService;
102+
private final ObjectPool<ScheduledExecutorService> timeServicePool;
104103
private final ScheduledExecutorService timeService;
105104
private final FilterRegistry filterRegistry;
106105
private final ThreadSafeRandom random = ThreadSafeRandomImpl.instance;
@@ -128,14 +127,16 @@ public void uncaughtException(Thread t, Throwable e) {
128127
// NamedFilterConfig.filterStateKey -> filter_instance.
129128
private final HashMap<String, Filter> activeFiltersDefaultChain = new HashMap<>();
130129

130+
@VisibleForTesting
131131
XdsServerWrapper(
132-
String listenerAddress,
133-
ServerBuilder<?> delegateBuilder,
134-
XdsServingStatusListener listener,
135-
FilterChainSelectorManager filterChainSelectorManager,
136-
XdsClientPoolFactory xdsClientPoolFactory,
137-
@Nullable Map<String, ?> bootstrapOverride,
138-
FilterRegistry filterRegistry) {
132+
String listenerAddress,
133+
ServerBuilder<?> delegateBuilder,
134+
XdsServingStatusListener listener,
135+
FilterChainSelectorManager filterChainSelectorManager,
136+
XdsClientPoolFactory xdsClientPoolFactory,
137+
@Nullable Map<String, ?> bootstrapOverride,
138+
FilterRegistry filterRegistry,
139+
ScheduledExecutorService timeService) {
139140
this(
140141
listenerAddress,
141142
delegateBuilder,
@@ -144,11 +145,9 @@ public void uncaughtException(Thread t, Throwable e) {
144145
xdsClientPoolFactory,
145146
bootstrapOverride,
146147
filterRegistry,
147-
SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE));
148-
sharedTimeService = true;
148+
new FixedObjectPool<>(timeService));
149149
}
150150

151-
@VisibleForTesting
152151
XdsServerWrapper(
153152
String listenerAddress,
154153
ServerBuilder<?> delegateBuilder,
@@ -157,7 +156,7 @@ public void uncaughtException(Thread t, Throwable e) {
157156
XdsClientPoolFactory xdsClientPoolFactory,
158157
@Nullable Map<String, ?> bootstrapOverride,
159158
FilterRegistry filterRegistry,
160-
ScheduledExecutorService timeService) {
159+
ObjectPool<ScheduledExecutorService> timeServicePool) {
161160
this.listenerAddress = checkNotNull(listenerAddress, "listenerAddress");
162161
this.delegateBuilder = checkNotNull(delegateBuilder, "delegateBuilder");
163162
this.delegateBuilder.intercept(new ConfigApplyingInterceptor());
@@ -166,7 +165,8 @@ public void uncaughtException(Thread t, Throwable e) {
166165
= checkNotNull(filterChainSelectorManager, "filterChainSelectorManager");
167166
this.xdsClientPoolFactory = checkNotNull(xdsClientPoolFactory, "xdsClientPoolFactory");
168167
this.bootstrapOverride = bootstrapOverride;
169-
this.timeService = checkNotNull(timeService, "timeService");
168+
this.timeServicePool = checkNotNull(timeServicePool, "timeServicePool");
169+
this.timeService = checkNotNull(timeServicePool.getObject(), "timeService");
170170
this.filterRegistry = checkNotNull(filterRegistry,"filterRegistry");
171171
this.delegate = delegateBuilder.build();
172172
}
@@ -275,9 +275,7 @@ private void internalShutdown() {
275275
if (restartTimer != null) {
276276
restartTimer.cancel();
277277
}
278-
if (sharedTimeService) {
279-
SharedResourceHolder.release(GrpcUtil.TIMER_SERVICE, timeService);
280-
}
278+
timeServicePool.returnObject(timeService);
281279
isServing = false;
282280
internalTerminationLatch.countDown();
283281
}

‎xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import io.grpc.Status;
3939
import io.grpc.StatusOr;
4040
import io.grpc.inprocess.InProcessSocketAddress;
41+
import io.grpc.internal.FakeClock;
4142
import io.grpc.internal.TestUtils.NoopChannelLogger;
4243
import io.grpc.netty.GrpcHttp2ConnectionHandler;
4344
import io.grpc.netty.InternalProtocolNegotiationEvent;
@@ -122,7 +123,8 @@ public void setUp() {
122123
when(mockServer.isShutdown()).thenReturn(false);
123124
xdsServerWrapper = new XdsServerWrapper("0.0.0.0:" + PORT, mockBuilder, listener,
124125
selectorManager, new FakeXdsClientPoolFactory(xdsClient),
125-
XdsServerTestHelper.RAW_BOOTSTRAP, FilterRegistry.newRegistry());
126+
XdsServerTestHelper.RAW_BOOTSTRAP, FilterRegistry.newRegistry(),
127+
new FakeClock().getScheduledExecutorService());
126128
}
127129

128130
@Test

‎xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ public void testBootstrap() throws Exception {
160160
when(xdsClient.getBootstrapInfo()).thenReturn(b);
161161
xdsServerWrapper = new XdsServerWrapper("[::FFFF:129.144.52.38]:80", mockBuilder, listener,
162162
selectorManager, new FakeXdsClientPoolFactory(xdsClient),
163-
XdsServerTestHelper.RAW_BOOTSTRAP, filterRegistry);
163+
XdsServerTestHelper.RAW_BOOTSTRAP, filterRegistry, executor.getScheduledExecutorService());
164164
Executors.newSingleThreadExecutor().execute(new Runnable() {
165165
@Override
166166
public void run() {
@@ -194,7 +194,8 @@ private void verifyBootstrapFail(Bootstrapper.BootstrapInfo b) throws Exception
194194
when(xdsClient.getBootstrapInfo()).thenReturn(b);
195195
xdsServerWrapper = new XdsServerWrapper("0.0.0.0:1", mockBuilder, listener,
196196
selectorManager, new FakeXdsClientPoolFactory(xdsClient),
197-
XdsServerTestHelper.RAW_BOOTSTRAP, filterRegistry);
197+
XdsServerTestHelper.RAW_BOOTSTRAP, filterRegistry,
198+
executor.getScheduledExecutorService());
198199
final SettableFuture<Server> start = SettableFuture.create();
199200
Executors.newSingleThreadExecutor().execute(new Runnable() {
200201
@Override
@@ -234,7 +235,8 @@ public void testBootstrap_templateWithXdstp() throws Exception {
234235
when(xdsClient.getBootstrapInfo()).thenReturn(b);
235236
xdsServerWrapper = new XdsServerWrapper("[::FFFF:129.144.52.38]:80", mockBuilder, listener,
236237
selectorManager, new FakeXdsClientPoolFactory(xdsClient),
237-
XdsServerTestHelper.RAW_BOOTSTRAP, filterRegistry);
238+
XdsServerTestHelper.RAW_BOOTSTRAP, filterRegistry,
239+
executor.getScheduledExecutorService());
238240
Executors.newSingleThreadExecutor().execute(new Runnable() {
239241
@Override
240242
public void run() {
@@ -1859,7 +1861,8 @@ private FilterRegistry filterStateTestFilterRegistry(
18591861
private SettableFuture<Server> filterStateTestStartServer(FilterRegistry filterRegistry) {
18601862
xdsServerWrapper = new XdsServerWrapper("0.0.0.0:1", mockBuilder, listener,
18611863
selectorManager, new FakeXdsClientPoolFactory(xdsClient),
1862-
XdsServerTestHelper.RAW_BOOTSTRAP, filterRegistry);
1864+
XdsServerTestHelper.RAW_BOOTSTRAP, filterRegistry,
1865+
executor.getScheduledExecutorService());
18631866
SettableFuture<Server> serverStart = SettableFuture.create();
18641867
scheduleServerStart(xdsServerWrapper, serverStart);
18651868
return serverStart;

0 commit comments

Comments
 (0)