Skip to content

Commit e04164b

Browse files
committed
Updated verifyResourceMetadataNacked method to verify ResourceMetadata.cached value
1 parent 80a9c40 commit e04164b

File tree

4 files changed

+14
-124
lines changed

4 files changed

+14
-124
lines changed

xds/src/main/java/io/grpc/xds/client/XdsClient.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,8 @@ public static ResourceMetadata newResourceMetadataUnknown() {
174174
return new ResourceMetadata(ResourceMetadataStatus.UNKNOWN, "", 0, false,null, null);
175175
}
176176

177-
public static ResourceMetadata newResourceMetadataRequested(boolean cached) {
178-
return new ResourceMetadata(ResourceMetadataStatus.REQUESTED, "", 0, cached, null, null);
177+
public static ResourceMetadata newResourceMetadataRequested() {
178+
return new ResourceMetadata(ResourceMetadataStatus.REQUESTED, "", 0, false, null, null);
179179
}
180180

181181
public static ResourceMetadata newResourceMetadataDoesNotExist() {

xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ public String toString() {
635635
}
636636

637637
// Initial fetch scheduled or rescheduled, transition metadata state to REQUESTED.
638-
metadata = ResourceMetadata.newResourceMetadataRequested(this.data != null);
638+
metadata = ResourceMetadata.newResourceMetadataRequested();
639639

640640
respTimer = syncContext.schedule(
641641
new ResourceNotFound(), INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS,

xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java

Lines changed: 9 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@
107107
import java.util.ArrayDeque;
108108
import java.util.Arrays;
109109
import java.util.Collections;
110-
import java.util.HashMap;
111110
import java.util.List;
112-
import java.util.Locale;
113111
import java.util.Map;
114112
import java.util.Queue;
115113
import java.util.concurrent.BlockingDeque;
@@ -487,10 +485,11 @@ private void verifyResourceMetadataAcked(
487485
private void verifyResourceMetadataNacked(
488486
XdsResourceType<?> type, String resourceName, Any rawResource, String versionInfo,
489487
long updateTime, String failedVersion, long failedUpdateTimeNanos,
490-
List<String> failedDetails) {
488+
List<String> failedDetails, boolean cached) {
491489
ResourceMetadata resourceMetadata =
492490
verifyResourceMetadata(type, resourceName, rawResource, ResourceMetadataStatus.NACKED,
493491
versionInfo, updateTime, true);
492+
assertThat(resourceMetadata.isCached()).isEqualTo(cached);
494493

495494
UpdateFailureState errorState = resourceMetadata.getErrorState();
496495
assertThat(errorState).isNotNull();
@@ -634,30 +633,6 @@ private void verifyServerFailureCount(int times, long serverFailureCount, String
634633
eq(xdsServer));
635634
}
636635

637-
private void verifyResourceCountByCacheState(XdsResourceType<?> type, String cacheState,
638-
long resourceCount) {
639-
Map<String, ResourceMetadata> metadataMap = awaitSubscribedResourcesMetadata().get(type);
640-
assertThat(metadataMap).isNotNull();
641-
642-
// Calculate number of resources in cacheState
643-
Map<String, Long> resourceCountsByState = new HashMap<>();
644-
for (ResourceMetadata metadata : metadataMap.values()) {
645-
String cacheStateFromResourceStatus = cacheStateFromResourceStatus(metadata.getStatus(),
646-
metadata.isCached());
647-
resourceCountsByState.compute(cacheStateFromResourceStatus,
648-
(k, v) -> (v == null) ? 1 : v + 1);
649-
}
650-
651-
assertThat(resourceCountsByState.get(cacheState)).isEqualTo(resourceCount);
652-
}
653-
654-
private String cacheStateFromResourceStatus(ResourceMetadataStatus metadataStatus,
655-
boolean isResourceCached) {
656-
String status = metadataStatus.toString().toLowerCase(Locale.ROOT);
657-
return metadataStatus == ResourceMetadataStatus.NACKED && isResourceCached
658-
? status + "_but_cached" : status;
659-
}
660-
661636
/**
662637
* Invokes the callback, which will be called by {@link XdsClientMetricReporter} to record
663638
* whether XdsClient has a working ADS stream.
@@ -803,7 +778,7 @@ public void ldsResponseErrorHandling_subscribedResourceInvalid() {
803778
List<String> errorsV2 = ImmutableList.of("LDS response Listener 'B' validation error: ");
804779
verifyResourceMetadataAcked(LDS, "A", resourcesV2.get("A"), VERSION_2, TIME_INCREMENT * 2);
805780
verifyResourceMetadataNacked(LDS, "B", resourcesV1.get("B"), VERSION_1, TIME_INCREMENT,
806-
VERSION_2, TIME_INCREMENT * 2, errorsV2);
781+
VERSION_2, TIME_INCREMENT * 2, errorsV2, true);
807782
// Check metric data.
808783
verifyResourceValidInvalidCount(1, 1, 1, xdsServerInfo.target(), LDS.typeUrl());
809784
if (!ignoreResourceDeletion()) {
@@ -896,7 +871,7 @@ public void ldsResponseErrorHandling_subscribedResourceInvalid_withRdsSubscripti
896871
verifyResourceMetadataAcked(LDS, "A", resourcesV2.get("A"), VERSION_2, TIME_INCREMENT * 3);
897872
verifyResourceMetadataNacked(
898873
LDS, "B", resourcesV1.get("B"), VERSION_1, TIME_INCREMENT, VERSION_2, TIME_INCREMENT * 3,
899-
errorsV2);
874+
errorsV2, true);
900875
if (!ignoreResourceDeletion()) {
901876
verifyResourceMetadataDoesNotExist(LDS, "C");
902877
} else {
@@ -1579,7 +1554,7 @@ public void rdsResponseErrorHandling_subscribedResourceInvalid() {
15791554
ImmutableList.of("RDS response RouteConfiguration 'B' validation error: ");
15801555
verifyResourceMetadataAcked(RDS, "A", resourcesV2.get("A"), VERSION_2, TIME_INCREMENT * 2);
15811556
verifyResourceMetadataNacked(RDS, "B", resourcesV1.get("B"), VERSION_1, TIME_INCREMENT,
1582-
VERSION_2, TIME_INCREMENT * 2, errorsV2);
1557+
VERSION_2, TIME_INCREMENT * 2, errorsV2, true);
15831558
verifyResourceMetadataAcked(RDS, "C", resourcesV1.get("C"), VERSION_1, TIME_INCREMENT);
15841559
call.verifyRequestNack(RDS, subscribedResourceNames, VERSION_1, "0001", NODE, errorsV2);
15851560

@@ -1963,9 +1938,9 @@ public void cdsResponseErrorHandling_subscribedResourceInvalid() {
19631938
List<String> errorsV2 = ImmutableList.of("CDS response Cluster 'B' validation error: ");
19641939
verifyResourceMetadataAcked(CDS, "A", resourcesV2.get("A"), VERSION_2, TIME_INCREMENT * 2);
19651940
verifyResourceMetadataNacked(CDS, "B", resourcesV1.get("B"), VERSION_1, TIME_INCREMENT,
1966-
VERSION_2, TIME_INCREMENT * 2, errorsV2);
1941+
VERSION_2, TIME_INCREMENT * 2, errorsV2, true);
19671942
if (!ignoreResourceDeletion()) {
1968-
verifyResourceMetadataDoesNotExist(CDS, "C");;
1943+
verifyResourceMetadataDoesNotExist(CDS, "C");
19691944
} else {
19701945
// When resource deletion is disabled, {C} stays ACKed in the previous version VERSION_1.
19711946
verifyResourceMetadataAcked(CDS, "C", resourcesV1.get("C"), VERSION_1, TIME_INCREMENT);
@@ -2068,7 +2043,7 @@ public void cdsResponseErrorHandling_subscribedResourceInvalid_withEdsSubscripti
20682043
verifyResourceMetadataAcked(CDS, "A", resourcesV2.get("A"), VERSION_2, TIME_INCREMENT * 3);
20692044
verifyResourceMetadataNacked(
20702045
CDS, "B", resourcesV1.get("B"), VERSION_1, TIME_INCREMENT, VERSION_2, TIME_INCREMENT * 3,
2071-
errorsV2);
2046+
errorsV2, true);
20722047
if (!ignoreResourceDeletion()) {
20732048
verifyResourceMetadataDoesNotExist(CDS, "C");
20742049
} else {
@@ -2991,7 +2966,7 @@ public void edsResponseErrorHandling_subscribedResourceInvalid() {
29912966
ImmutableList.of("EDS response ClusterLoadAssignment 'B' validation error: ");
29922967
verifyResourceMetadataAcked(EDS, "A", resourcesV2.get("A"), VERSION_2, TIME_INCREMENT * 2);
29932968
verifyResourceMetadataNacked(EDS, "B", resourcesV1.get("B"), VERSION_1, TIME_INCREMENT,
2994-
VERSION_2, TIME_INCREMENT * 2, errorsV2);
2969+
VERSION_2, TIME_INCREMENT * 2, errorsV2, true);
29952970
verifyResourceMetadataAcked(EDS, "C", resourcesV1.get("C"), VERSION_1, TIME_INCREMENT);
29962971
call.verifyRequestNack(EDS, subscribedResourceNames, VERSION_1, "0001", NODE, errorsV2);
29972972

@@ -4126,90 +4101,6 @@ public void serverFailureMetricReport_forRetryAndBackoff() {
41264101
}
41274102

41284103

4129-
@Test
4130-
public void testResourceCounts() {
4131-
List<String> subscribedResourceNames = ImmutableList.of("A", "B", "C");
4132-
xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "A", ldsResourceWatcher);
4133-
xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "B", ldsResourceWatcher);
4134-
xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "C", ldsResourceWatcher);
4135-
DiscoveryRpcCall call = resourceDiscoveryCalls.poll();
4136-
assertThat(call).isNotNull();
4137-
verifyResourceMetadataRequested(LDS, "A");
4138-
verifyResourceMetadataRequested(LDS, "B");
4139-
verifyResourceMetadataRequested(LDS, "C");
4140-
verifySubscribedResourcesMetadataSizes(3, 0, 0, 0);
4141-
// Check metric data.
4142-
verifyResourceCountByCacheState(LDS,"requested", 3);
4143-
4144-
// LDS -> {A, B, C}, version 1
4145-
ImmutableMap<String, Any> resourcesV1 = ImmutableMap.of(
4146-
"A", Any.pack(mf.buildListenerWithApiListenerForRds("A", "A.1")),
4147-
"B", Any.pack(mf.buildListenerWithApiListenerForRds("B", "B.1")),
4148-
"C", Any.pack(mf.buildListenerWithApiListenerForRds("C", "C.1")));
4149-
call.sendResponse(LDS, resourcesV1.values().asList(), VERSION_1, "0000");
4150-
// {A, B, C} -> ACK, version 1
4151-
verifyResourceValidInvalidCount(1, 3, 0, xdsServerInfo.target(), LDS.typeUrl());
4152-
verifyResourceMetadataAcked(LDS, "A", resourcesV1.get("A"), VERSION_1, TIME_INCREMENT);
4153-
verifyResourceMetadataAcked(LDS, "B", resourcesV1.get("B"), VERSION_1, TIME_INCREMENT);
4154-
verifyResourceMetadataAcked(LDS, "C", resourcesV1.get("C"), VERSION_1, TIME_INCREMENT);
4155-
// Check metric data.
4156-
verifyResourceCountByCacheState(LDS, "acked",3);
4157-
call.verifyRequest(LDS, subscribedResourceNames, VERSION_1, "0000", NODE);
4158-
4159-
// LDS -> {A, B}, version 2
4160-
// Failed to parse endpoint B
4161-
ImmutableMap<String, Any> resourcesV2 = ImmutableMap.of(
4162-
"A", Any.pack(mf.buildListenerWithApiListenerForRds("A", "A.2")),
4163-
"B", Any.pack(mf.buildListenerWithApiListenerInvalid("B")));
4164-
call.sendResponse(LDS, resourcesV2.values().asList(), VERSION_2, "0001");
4165-
// {A} -> ACK, version 2
4166-
// {B} -> NACK, version 1, rejected version 2, rejected reason: Failed to parse B
4167-
// {C} -> does not exist
4168-
verifyResourceValidInvalidCount(1, 1, 1, xdsServerInfo.target(), LDS.typeUrl());
4169-
List<String> errorsV2 = ImmutableList.of("LDS response Listener 'B' validation error: ");
4170-
verifyResourceMetadataAcked(LDS, "A", resourcesV2.get("A"), VERSION_2, TIME_INCREMENT * 2);
4171-
verifyResourceMetadataNacked(LDS, "B", resourcesV1.get("B"), VERSION_1, TIME_INCREMENT,
4172-
VERSION_2, TIME_INCREMENT * 2, errorsV2);
4173-
if (!ignoreResourceDeletion()) {
4174-
verifyResourceMetadataDoesNotExist(LDS, "C");
4175-
// Check metric data.
4176-
verifyResourceCountByCacheState(LDS, "acked", 1);
4177-
verifyResourceCountByCacheState(LDS, "nacked_but_cached", 1);
4178-
verifyResourceCountByCacheState(LDS, "does_not_exist", 1);
4179-
} else {
4180-
// When resource deletion is disabled, {C} stays ACKed in the previous version VERSION_1.
4181-
verifyResourceMetadataAcked(LDS, "C", resourcesV1.get("C"), VERSION_1, TIME_INCREMENT);
4182-
// Check metric data.
4183-
verifyResourceCountByCacheState(LDS, "acked", 2);
4184-
verifyResourceCountByCacheState(LDS, "nacked_but_cached", 1);
4185-
}
4186-
call.verifyRequestNack(LDS, subscribedResourceNames, VERSION_1, "0001", NODE, errorsV2);
4187-
4188-
// LDS -> {B, C} version 3
4189-
ImmutableMap<String, Any> resourcesV3 = ImmutableMap.of(
4190-
"B", Any.pack(mf.buildListenerWithApiListenerForRds("B", "B.3")),
4191-
"C", Any.pack(mf.buildListenerWithApiListenerForRds("C", "C.3")));
4192-
call.sendResponse(LDS, resourcesV3.values().asList(), VERSION_3, "0002");
4193-
// {A} -> does not exist
4194-
// {B, C} -> ACK, version 3
4195-
verifyResourceValidInvalidCount(1, 2, 0, xdsServerInfo.target(), LDS.typeUrl());
4196-
if (!ignoreResourceDeletion()) {
4197-
verifyResourceMetadataDoesNotExist(LDS, "A");
4198-
// Check metric data.
4199-
verifyResourceCountByCacheState(LDS, "acked", 2);
4200-
verifyResourceCountByCacheState(LDS, "does_not_exist", 1);
4201-
} else {
4202-
// When resource deletion is disabled, {A} stays ACKed in the previous version VERSION_2.
4203-
verifyResourceMetadataAcked(LDS, "A", resourcesV2.get("A"), VERSION_2, TIME_INCREMENT * 2);
4204-
// Check metric data.
4205-
verifyResourceCountByCacheState(LDS, "acked", 3);
4206-
}
4207-
verifyResourceMetadataAcked(LDS, "B", resourcesV3.get("B"), VERSION_3, TIME_INCREMENT * 3);
4208-
verifyResourceMetadataAcked(LDS, "C", resourcesV3.get("C"), VERSION_3, TIME_INCREMENT * 3);
4209-
call.verifyRequest(LDS, subscribedResourceNames, VERSION_3, "0002", NODE);
4210-
verifySubscribedResourcesMetadataSizes(3, 0, 0, 0);
4211-
}
4212-
42134104
private XdsClientImpl createXdsClient(String serverUri) {
42144105
BootstrapInfo bootstrapInfo = buildBootStrap(serverUri);
42154106
return new XdsClientImpl(

xds/src/test/java/io/grpc/xds/XdsClientMetricReporterImplTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ public void reportCallbackMetrics_computeAndReportResourceCounts() {
241241

242242
Map<String, ResourceMetadata> ldsResourceMetadataMap = new HashMap<>();
243243
ldsResourceMetadataMap.put("resource1",
244-
ResourceMetadata.newResourceMetadataRequested(false));
244+
ResourceMetadata.newResourceMetadataRequested());
245245
ResourceMetadata ackedLdsResource = ResourceMetadata.newResourceMetadataAcked(rawListener, "42",
246246
nanosLastUpdate);
247247
ldsResourceMetadataMap.put("resource2", ackedLdsResource);
@@ -252,8 +252,7 @@ public void reportCallbackMetrics_computeAndReportResourceCounts() {
252252
"nacked after previous ack", true));
253253

254254
Map<String, ResourceMetadata> rdsResourceMetadataMap = new HashMap<>();
255-
ResourceMetadata requestedRdsResourceMetadata = ResourceMetadata.newResourceMetadataRequested(
256-
false);
255+
ResourceMetadata requestedRdsResourceMetadata = ResourceMetadata.newResourceMetadataRequested();
257256
rdsResourceMetadataMap.put("resource5",
258257
ResourceMetadata.newResourceMetadataNacked(requestedRdsResourceMetadata, "24",
259258
nanosLastUpdate, "nacked after request", false));

0 commit comments

Comments
 (0)