Skip to content

Commit 7bb39c7

Browse files
bryce-andersonbryce-anderson
committed
Activate backup requests via property
1 parent 7e14cb9 commit 7bb39c7

File tree

3 files changed

+184
-113
lines changed

3 files changed

+184
-113
lines changed

‎servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsClient.java‎

Lines changed: 112 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -147,112 +147,6 @@ final class DefaultDnsClient implements DnsClient {
147147
private final String id;
148148
private boolean closed;
149149

150-
private interface DnsNameResolverDelegate {
151-
void close();
152-
153-
Future<List<InetAddress>> resolveAll(String name);
154-
155-
Future<List<DnsRecord>> resolveAll(DnsQuestion name);
156-
157-
long queryTimeoutMillis();
158-
}
159-
160-
private static final class SingleResolver implements DnsNameResolverDelegate {
161-
private final DnsNameResolver nettyResolver;
162-
163-
SingleResolver(DnsNameResolver nettyResolver) {
164-
this.nettyResolver = nettyResolver;
165-
}
166-
167-
@Override
168-
public void close() {
169-
nettyResolver.close();
170-
}
171-
172-
@Override
173-
public Future<List<InetAddress>> resolveAll(String name) {
174-
return nettyResolver.resolveAll(name);
175-
}
176-
177-
@Override
178-
public Future<List<DnsRecord>> resolveAll(DnsQuestion question) {
179-
return nettyResolver.resolveAll(question);
180-
}
181-
182-
@Override
183-
public long queryTimeoutMillis() {
184-
return nettyResolver.queryTimeoutMillis();
185-
}
186-
}
187-
188-
private static final class BackupRequestResolver implements DnsNameResolverDelegate {
189-
190-
private final DnsNameResolver primaryResolver;
191-
private final DnsNameResolver backupResolver;
192-
private final EventLoop eventLoop;
193-
194-
// TODO: we'll want to make sure we share the cache, make our backup request interval more flexible, and also
195-
// give ourselves some sort of budget so we don't overload DNS, but these are all possible.
196-
BackupRequestResolver(DnsNameResolverBuilder builder, EventLoop eventLoop) {
197-
primaryResolver = builder.build();
198-
backupResolver = builder.consolidateCacheSize(0).build();
199-
this.eventLoop = eventLoop;
200-
}
201-
202-
@Override
203-
public void close() {
204-
try {
205-
primaryResolver.close();
206-
} finally {
207-
backupResolver.close();
208-
}
209-
}
210-
211-
@Override
212-
public Future<List<InetAddress>> resolveAll(String name) {
213-
return withBackup(resolver -> resolver.resolveAll(name));
214-
}
215-
216-
@Override
217-
public Future<List<DnsRecord>> resolveAll(DnsQuestion name) {
218-
return withBackup(resolver -> resolver.resolveAll(name));
219-
}
220-
221-
@Override
222-
public long queryTimeoutMillis() {
223-
return primaryResolver.queryTimeoutMillis();
224-
}
225-
226-
private <T> Future<T> withBackup(Function<? super DnsNameResolver, ? extends Future<T>> query) {
227-
Future<T> primaryQuery = query.apply(primaryResolver);
228-
if (primaryQuery.isDone()) {
229-
return primaryQuery;
230-
}
231-
int backupDelay = backupDelayMs();
232-
if (backupDelay <= 0) {
233-
// no backup for this request
234-
return primaryQuery;
235-
}
236-
Promise<T> result = eventLoop.newPromise();
237-
Future<?> timer = eventLoop.schedule(() -> {
238-
if (allowBackupRequest()) {
239-
PromiseNotifier.cascade(false, query.apply(backupResolver), result);
240-
}
241-
}, backupDelay, MILLISECONDS);
242-
primaryQuery.addListener(_unused -> timer.cancel(true));
243-
PromiseNotifier.cascade(false, primaryQuery, result);
244-
return result;
245-
}
246-
247-
private boolean allowBackupRequest() {
248-
return true;
249-
}
250-
251-
private int backupDelayMs() {
252-
return 50;
253-
}
254-
}
255-
256150
DefaultDnsClient(final String id, final IoExecutor ioExecutor, final int consolidateCacheSize,
257151
final int minTTL, final int maxTTL, final int minCacheTTL, final int maxCacheTTL,
258152
final int negativeTTLCacheSeconds, final long ttlJitterNanos,
@@ -269,7 +163,8 @@ private int backupDelayMs() {
269163
final ServiceDiscovererEvent.Status missingRecordStatus,
270164
final boolean nxInvalidation,
271165
final boolean tcpFallbackOnTimeout,
272-
final String datagramChannelStrategy) {
166+
final String datagramChannelStrategy,
167+
@Nullable final Integer backupRequestDelay) {
273168
this.srvConcurrency = srvConcurrency;
274169
this.srvFilterDuplicateEvents = srvFilterDuplicateEvents;
275170
// Implementation of this class expects to use only single EventLoop from IoExecutor
@@ -338,8 +233,9 @@ private int backupDelayMs() {
338233
if (dnsServerAddressStreamProvider != null) {
339234
builder.nameServerProvider(toNettyType(dnsServerAddressStreamProvider));
340235
}
341-
// resolver = new SingleResolver(builder.build());
342-
resolver = new BackupRequestResolver(builder, eventLoop);
236+
resolver = backupRequestDelay == null || backupRequestDelay <= 0 ? new DefaultResolver(builder.build()) :
237+
new BackupRequestResolver(builder.build(), builder.consolidateCacheSize(0).build(),
238+
eventLoop, backupRequestDelay);
343239
this.resolutionTimeoutMillis = resolutionTimeout != null ? resolutionTimeout.toMillis() :
344240
// Default value is chosen based on a combination of default "timeout" and "attempts" options of
345241
// /etc/resolv.conf: https://man7.org/linux/man-pages/man5/resolv.conf.5.html
@@ -1259,4 +1155,111 @@ static SrvAddressRemovedException newInstance(Class<?> clazz, String method) {
12591155
return unknownStackTrace(new SrvAddressRemovedException(), clazz, method);
12601156
}
12611157
}
1158+
1159+
interface DnsNameResolverDelegate {
1160+
void close();
1161+
1162+
Future<List<InetAddress>> resolveAll(String name);
1163+
1164+
Future<List<DnsRecord>> resolveAll(DnsQuestion name);
1165+
1166+
long queryTimeoutMillis();
1167+
}
1168+
1169+
static final class DefaultResolver implements DnsNameResolverDelegate {
1170+
private final DnsNameResolver nettyResolver;
1171+
1172+
DefaultResolver(DnsNameResolver nettyResolver) {
1173+
this.nettyResolver = nettyResolver;
1174+
}
1175+
1176+
@Override
1177+
public void close() {
1178+
nettyResolver.close();
1179+
}
1180+
1181+
@Override
1182+
public Future<List<InetAddress>> resolveAll(String name) {
1183+
return nettyResolver.resolveAll(name);
1184+
}
1185+
1186+
@Override
1187+
public Future<List<DnsRecord>> resolveAll(DnsQuestion question) {
1188+
return nettyResolver.resolveAll(question);
1189+
}
1190+
1191+
@Override
1192+
public long queryTimeoutMillis() {
1193+
return nettyResolver.queryTimeoutMillis();
1194+
}
1195+
}
1196+
1197+
static final class BackupRequestResolver implements DnsNameResolverDelegate {
1198+
1199+
private final DnsNameResolver primaryResolver;
1200+
private final DnsNameResolver backupResolver;
1201+
private final EventLoop eventLoop;
1202+
private final int backupDelayMs;
1203+
1204+
BackupRequestResolver(DnsNameResolver primaryResolver, DnsNameResolver backupResolver, EventLoop eventLoop, int backupDelayMs) {
1205+
this.primaryResolver = primaryResolver;
1206+
this.backupResolver = backupResolver;
1207+
this.eventLoop = eventLoop;
1208+
this.backupDelayMs = backupDelayMs;
1209+
}
1210+
1211+
@Override
1212+
public void close() {
1213+
try {
1214+
primaryResolver.close();
1215+
} finally {
1216+
backupResolver.close();
1217+
}
1218+
}
1219+
1220+
@Override
1221+
public Future<List<InetAddress>> resolveAll(String name) {
1222+
return withBackup(resolver -> resolver.resolveAll(name));
1223+
}
1224+
1225+
@Override
1226+
public Future<List<DnsRecord>> resolveAll(DnsQuestion name) {
1227+
return withBackup(resolver -> resolver.resolveAll(name));
1228+
}
1229+
1230+
@Override
1231+
public long queryTimeoutMillis() {
1232+
return primaryResolver.queryTimeoutMillis();
1233+
}
1234+
1235+
private <T> Future<T> withBackup(Function<? super DnsNameResolver, ? extends Future<T>> query) {
1236+
Future<T> primaryQuery = query.apply(primaryResolver);
1237+
if (primaryQuery.isDone()) {
1238+
return primaryQuery;
1239+
}
1240+
int backupDelay = backupDelayMs();
1241+
if (backupDelay <= 0) {
1242+
// no backup for this request
1243+
return primaryQuery;
1244+
}
1245+
Promise<T> result = eventLoop.newPromise();
1246+
Future<?> timer = eventLoop.schedule(() -> {
1247+
if (allowBackupRequest()) {
1248+
PromiseNotifier.cascade(false, query.apply(backupResolver), result);
1249+
}
1250+
}, backupDelay, MILLISECONDS);
1251+
primaryQuery.addListener(_unused -> timer.cancel(true));
1252+
PromiseNotifier.cascade(false, primaryQuery, result);
1253+
return result;
1254+
}
1255+
1256+
private boolean allowBackupRequest() {
1257+
// In the future we should make this predicated on a token bucket.
1258+
return true;
1259+
}
1260+
1261+
private int backupDelayMs() {
1262+
return backupDelayMs;
1263+
}
1264+
}
12621265
}

‎servicetalk-dns-discovery-netty/src/main/java/io/servicetalk/dns/discovery/netty/DefaultDnsServiceDiscovererBuilder.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import static io.servicetalk.utils.internal.NumberUtils.ensureNonNegative;
4242
import static io.servicetalk.utils.internal.NumberUtils.ensurePositive;
4343
import static java.lang.Boolean.getBoolean;
44+
import static java.lang.Integer.getInteger;
4445
import static java.lang.Math.min;
4546
import static java.lang.System.getProperty;
4647
import static java.time.Duration.ofSeconds;
@@ -58,6 +59,9 @@ public final class DefaultDnsServiceDiscovererBuilder implements DnsServiceDisco
5859

5960
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultDnsServiceDiscovererBuilder.class);
6061

62+
// Backup request static configuration: values > 0 mean allow a backup request with fixed delay, disabled otherwise.
63+
private static final String DNS_BACKUP_REQUEST_DELAY_MS_PROPERTY =
64+
"io.servicetalk.dns.discovery.netty.experimental.dnsBackupRequestDelayMs";
6165
// FIXME: 0.43 - consider removing deprecated system properties.
6266
// Those were introduced temporarily as a way for us to experiment with new Netty features.
6367
// In the next major release, we should promote required features to builder API.
@@ -72,6 +76,8 @@ public final class DefaultDnsServiceDiscovererBuilder implements DnsServiceDisco
7276
@Deprecated
7377
private static final String NX_DOMAIN_INVALIDATES_PROPERTY = "io.servicetalk.dns.discovery.nxdomain.invalidation";
7478

79+
@Nullable
80+
private static final Integer DNS_BACKUP_REQUEST_DELAY_MS = getInteger(DNS_BACKUP_REQUEST_DELAY_MS_PROPERTY);
7581
private static final String DEFAULT_DATAGRAM_CHANNEL_STRATEGY =
7682
getProperty(DATAGRAM_CHANNEL_STRATEGY_PROPERTY, "ChannelPerResolver");
7783
private static final boolean DEFAULT_TCP_FALLBACK_ON_TIMEOUT = getBoolean(TCP_FALLBACK_ON_TIMEOUT_PROPERTY);
@@ -409,7 +415,7 @@ DnsClient build() {
409415
srvHostNameRepeatInitialDelay, srvHostNameRepeatJitter, maxUdpPayloadSize, ndots, optResourceEnabled,
410416
queryTimeout, resolutionTimeout, dnsResolverAddressTypes, localAddress, dnsServerAddressStreamProvider,
411417
observer, missingRecordStatus, nxInvalidation,
412-
DEFAULT_TCP_FALLBACK_ON_TIMEOUT, DEFAULT_DATAGRAM_CHANNEL_STRATEGY);
418+
DEFAULT_TCP_FALLBACK_ON_TIMEOUT, DEFAULT_DATAGRAM_CHANNEL_STRATEGY, DNS_BACKUP_REQUEST_DELAY_MS);
413419
return filterFactory == null ? rawClient : filterFactory.create(rawClient);
414420
}
415421

‎servicetalk-dns-discovery-netty/src/test/java/io/servicetalk/dns/discovery/netty/DefaultDnsClientTest.java‎

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,11 @@
3131
import io.servicetalk.transport.netty.internal.EventLoopAwareNettyIoExecutor;
3232
import io.servicetalk.utils.internal.DurationUtils;
3333

34+
import io.netty.channel.EventLoop;
3435
import io.netty.channel.EventLoopGroup;
36+
import io.netty.resolver.dns.DnsNameResolver;
37+
import io.netty.resolver.dns.DnsNameResolverBuilder;
38+
import io.netty.util.concurrent.Promise;
3539
import org.apache.directory.server.dns.messages.RecordType;
3640
import org.junit.jupiter.api.AfterEach;
3741
import org.junit.jupiter.api.Test;
@@ -41,6 +45,7 @@
4145
import org.junit.jupiter.params.provider.EnumSource;
4246
import org.junit.jupiter.params.provider.MethodSource;
4347
import org.junit.jupiter.params.provider.ValueSource;
48+
import org.mockito.ArgumentMatchers;
4449
import org.slf4j.Logger;
4550
import org.slf4j.LoggerFactory;
4651

@@ -56,6 +61,7 @@
5661
import java.util.concurrent.BlockingQueue;
5762
import java.util.concurrent.Callable;
5863
import java.util.concurrent.CountDownLatch;
64+
import java.util.concurrent.Future;
5965
import java.util.concurrent.RejectedExecutionException;
6066
import java.util.concurrent.TimeUnit;
6167
import java.util.concurrent.atomic.AtomicInteger;
@@ -94,11 +100,16 @@
94100
import static org.hamcrest.Matchers.instanceOf;
95101
import static org.hamcrest.Matchers.is;
96102
import static org.hamcrest.Matchers.nullValue;
103+
import static org.junit.jupiter.api.Assertions.assertEquals;
104+
import static org.junit.jupiter.api.Assertions.assertFalse;
97105
import static org.junit.jupiter.api.Assertions.assertNotNull;
98106
import static org.junit.jupiter.api.Assertions.assertNull;
99107
import static org.mockito.ArgumentMatchers.any;
100108
import static org.mockito.Mockito.doAnswer;
101109
import static org.mockito.Mockito.mock;
110+
import static org.mockito.Mockito.times;
111+
import static org.mockito.Mockito.verify;
112+
import static org.mockito.Mockito.when;
102113

103114
class DefaultDnsClientTest {
104115
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultDnsClientTest.class);
@@ -145,9 +156,15 @@ void setup(UnaryOperator<DefaultDnsServiceDiscovererBuilder> builderFunction) th
145156

146157
@AfterEach
147158
public void tearDown() throws Exception {
148-
client.closeAsync().toFuture().get();
149-
dnsServer.stop();
150-
dnsServer2.stop();
159+
if (client != null) {
160+
client.closeAsync().toFuture().get();
161+
}
162+
if (dnsServer != null) {
163+
dnsServer.stop();
164+
}
165+
if (dnsServer2 != null) {
166+
dnsServer2.stop();
167+
}
151168
}
152169

153170
private static void advanceTime() throws Exception {
@@ -1225,6 +1242,51 @@ void testResolutionTimeout(RecordType recordType) throws Exception {
12251242
testTimeout(Duration.ZERO, DEFAULT_TIMEOUT, recordType);
12261243
}
12271244

1245+
@Test
1246+
void backupRequest() throws Exception {
1247+
DnsNameResolver primaryResolver = mock(DnsNameResolver.class);
1248+
DnsNameResolver backupResolver = mock(DnsNameResolver.class);
1249+
EventLoop eventLoop = ioExecutor.executor().eventLoopGroup().next();
1250+
Promise<List<InetAddress>> primaryPromise = eventLoop.newPromise();
1251+
when(primaryResolver.resolveAll("foo")).thenReturn(primaryPromise);
1252+
1253+
Promise<List<InetAddress>> backupPromise = eventLoop.newPromise();
1254+
when(backupResolver.resolveAll("foo")).thenReturn(backupPromise);
1255+
1256+
DefaultDnsClient.DnsNameResolverDelegate resolver = new DefaultDnsClient.BackupRequestResolver(primaryResolver, backupResolver, eventLoop, 20);
1257+
Future<List<InetAddress>> result = resolver.resolveAll("foo");
1258+
assertFalse(result.isDone());
1259+
verify(primaryResolver, times(1)).resolveAll("foo");
1260+
verify(backupResolver, times(0)).resolveAll("foo");
1261+
1262+
// Wait 20 milliseconds.
1263+
eventLoop.schedule(() -> {}, 20, MILLISECONDS).get();
1264+
1265+
verify(primaryResolver, times(1)).resolveAll("foo");
1266+
verify(backupResolver, times(1)).resolveAll("foo");
1267+
}
1268+
1269+
@Test
1270+
void noBackupRequestIfOriginalSucceeds() throws Exception {
1271+
DnsNameResolver primaryResolver = mock(DnsNameResolver.class);
1272+
DnsNameResolver backupResolver = mock(DnsNameResolver.class);
1273+
EventLoop eventLoop = ioExecutor.executor().eventLoopGroup().next();
1274+
Promise<List<InetAddress>> primaryPromise = eventLoop.newPromise();
1275+
when(primaryResolver.resolveAll("foo")).thenReturn(primaryPromise);
1276+
when(backupResolver.resolveAll("foo")).thenReturn(eventLoop.newPromise());
1277+
1278+
DefaultDnsClient.DnsNameResolverDelegate resolver = new DefaultDnsClient.BackupRequestResolver(primaryResolver, backupResolver, eventLoop, 20);
1279+
Future<List<InetAddress>> resolve = resolver.resolveAll("foo");
1280+
assertFalse(resolve.isDone());
1281+
List<InetAddress> result = new ArrayList<>();
1282+
primaryPromise.trySuccess(result);
1283+
assertEquals(result, resolve.get(20, SECONDS));
1284+
// Wait for the timeout duration to be sure we only get one call.
1285+
eventLoop.schedule(() -> {}, 20, MILLISECONDS).get();
1286+
verify(primaryResolver, times(1)).resolveAll("foo");
1287+
verify(backupResolver, times(0)).resolveAll("foo");
1288+
}
1289+
12281290
void testTimeout(Duration queryTimeout, Duration resolutionTimeout, RecordType recordType) throws Exception {
12291291
setup(builder -> builder
12301292
.queryTimeout(queryTimeout)

0 commit comments

Comments
 (0)