Skip to content

Commit

Permalink
Enable respect_dns_ttl by default for xDS cluster (#1055)
Browse files Browse the repository at this point in the history
Motivation:

Envoy refreshes DNS every 5 seconds regardless of DNS TTL. The frequent
refresh consumes additional CPU and memory resources and may put a heavy
load on DNS servers.

`respect_dns_ttl` is an option to let Envoy use the TTL of DNS respones.
envoyproxy/envoy#6876
Respecting DNS TTL seems more sensible behavior for Armeria xDS modules.

Modifications:

- Set `respect_dns_ttl` to true when creating an xDS cluster.
- Due to the limation of protobuf, `respect_dns_ttl` in a request is
ignored but the value still can be modified via xDS update API.

Result:

`respect_dns_ttl` is now enabled by default for xDS cluster.
  • Loading branch information
ikhoon authored Nov 8, 2024
1 parent 4d4af7e commit 3cf9011
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,20 @@ public void createCluster(CreateClusterRequest request, StreamObserver<Cluster>
}

final String clusterName = parent + CLUSTERS_DIRECTORY + clusterId;
// Ignore the specified name in the cluster and set the name
// with the format of "groups/{group}/clusters/{cluster}".
// https://github.com/aip-dev/google.aip.dev/blob/master/aip/general/0133.md#user-specified-ids
final Cluster cluster = request.getCluster().toBuilder().setName(clusterName).build();
final Cluster cluster
= request.getCluster()
.toBuilder()
// Ignore the specified name in the cluster and set the name with the format of
// "groups/{group}/clusters/{cluster}".
// https://github.com/aip-dev/google.aip.dev/blob/master/aip/general/0133.md#user-specified-ids
.setName(clusterName)
// Respect the DNS TTL would be more efficient in terms of DNS resolution.
// https://github.com/envoyproxy/envoy/issues/6876
// `respect_dns_ttl` is a `bool` field so it is not possible to check whether a value
// has not been set for the field. Until we create our own proto file, the value only
// can be set to false via the update API.
.setRespectDnsTtl(true)
.build();
xdsResourceManager.push(responseObserver, group, clusterName, CLUSTERS_DIRECTORY + clusterId + ".json",
"Create cluster: " + clusterName, cluster, currentAuthor(), true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ void createClusterViaHttp() throws Exception {
JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), clusterBuilder);
final Cluster actualCluster = clusterBuilder.build();
final String clusterName = "groups/foo/clusters/foo-cluster/1";
assertThat(actualCluster).isEqualTo(cluster.toBuilder().setName(clusterName).build());
assertThat(actualCluster).isEqualTo(cluster.toBuilder()
.setName(clusterName)
.setRespectDnsTtl(true)
.build());
checkResourceViaDiscoveryRequest(actualCluster, clusterName, true);

// Create the same cluster again.
Expand Down Expand Up @@ -149,11 +152,18 @@ void updateClusterViaHttp() throws Exception {
JSON_MESSAGE_MARSHALLER.mergeValue(response.contentUtf8(), clusterBuilder);
final Cluster actualCluster = clusterBuilder.build();
final String clusterName = "groups/foo/clusters/foo-cluster/2";
assertThat(actualCluster).isEqualTo(cluster.toBuilder().setName(clusterName).build());
assertThat(actualCluster).isEqualTo(cluster.toBuilder()
.setName(clusterName)
.setRespectDnsTtl(true)
.build());
checkResourceViaDiscoveryRequest(actualCluster, clusterName, true);

final Cluster updatingCluster = cluster.toBuilder().setConnectTimeout(
Duration.newBuilder().setSeconds(2).build()).setName(clusterName).build();
final Cluster updatingCluster = cluster.toBuilder()
.setConnectTimeout(
Duration.newBuilder().setSeconds(2).build())
.setName(clusterName)
.setRespectDnsTtl(false)
.build();
response = updateCluster("groups/foo", "foo-cluster/2", updatingCluster, dogma.httpClient());
assertOk(response);
final Cluster.Builder clusterBuilder2 = Cluster.newBuilder();
Expand All @@ -177,7 +187,10 @@ void deleteClusterViaHttp() throws Exception {
response = createCluster("groups/foo", "foo-cluster/3/4", cluster, dogma.httpClient());
assertOk(response);

final Cluster actualCluster = cluster.toBuilder().setName(clusterName).build();
final Cluster actualCluster = cluster.toBuilder()
.setName(clusterName)
.setRespectDnsTtl(true)
.build();
checkResourceViaDiscoveryRequest(actualCluster, clusterName, true);

// Add permission test.
Expand All @@ -204,7 +217,10 @@ void viaStub() {
.setClusterId("foo-cluster/5/6")
.setCluster(cluster).build());
final String clusterName = "groups/foo/clusters/foo-cluster/5/6";
assertThat(response).isEqualTo(cluster.toBuilder().setName(clusterName).build());
assertThat(response).isEqualTo(cluster.toBuilder()
.setName(clusterName)
.setRespectDnsTtl(true)
.build());

final Cluster updatingCluster = cluster.toBuilder().setConnectTimeout(
Duration.newBuilder().setSeconds(2).build()).setName(clusterName).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ static Cluster cluster(String clusterName, ConfigSource configSource,
.setEdsConfig(configSource)
.setServiceName(clusterName))
.setType(Cluster.DiscoveryType.EDS)
.setRespectDnsTtl(true)
.build();
}

Expand Down

0 comments on commit 3cf9011

Please sign in to comment.