From d142abbcb90af85f4ddc7153ee0aa2e6500bb89c Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Wed, 14 May 2025 18:48:32 +0530 Subject: [PATCH] add resource_timer_is_transient_failure server feature --- .../java/io/grpc/xds/client/Bootstrapper.java | 9 ++++++--- .../java/io/grpc/xds/client/BootstrapperImpl.java | 13 ++++++++++++- .../main/java/io/grpc/xds/client/XdsClient.java | 6 +++++- .../java/io/grpc/xds/client/XdsClientImpl.java | 15 +++++++++++++-- .../io/grpc/xds/GrpcXdsClientImplDataTest.java | 2 +- .../io/grpc/xds/GrpcXdsClientImplTestBase.java | 4 ++-- .../java/io/grpc/xds/XdsNameResolverTest.java | 4 ++-- .../xds/client/CommonBootstrapperTestUtils.java | 2 +- 8 files changed, 42 insertions(+), 13 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/client/Bootstrapper.java b/xds/src/main/java/io/grpc/xds/client/Bootstrapper.java index 90babd1e8d0..4fa75f6b335 100644 --- a/xds/src/main/java/io/grpc/xds/client/Bootstrapper.java +++ b/xds/src/main/java/io/grpc/xds/client/Bootstrapper.java @@ -63,17 +63,20 @@ public abstract static class ServerInfo { public abstract boolean isTrustedXdsServer(); + public abstract boolean resourceTimerIsTransientError(); + @VisibleForTesting public static ServerInfo create(String target, @Nullable Object implSpecificConfig) { - return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig, false, false); + return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig, + false, false, false); } @VisibleForTesting public static ServerInfo create( String target, Object implSpecificConfig, boolean ignoreResourceDeletion, - boolean isTrustedXdsServer) { + boolean isTrustedXdsServer, boolean resourceTimerIsTransientError) { return new AutoValue_Bootstrapper_ServerInfo(target, implSpecificConfig, - ignoreResourceDeletion, isTrustedXdsServer); + ignoreResourceDeletion, isTrustedXdsServer, resourceTimerIsTransientError); } } diff --git a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java index c00685f1781..46708a95afb 100644 --- a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java @@ -43,6 +43,8 @@ public abstract class BootstrapperImpl extends Bootstrapper { public static final String GRPC_EXPERIMENTAL_XDS_FALLBACK = "GRPC_EXPERIMENTAL_XDS_FALLBACK"; + public static final String GRPC_EXPERIMENTAL_XDS_DATA_ERROR_HANDLING = + "GRPC_EXPERIMENTAL_XDS_DATA_ERROR_HANDLING"; // Client features. @VisibleForTesting @@ -54,10 +56,15 @@ public abstract class BootstrapperImpl extends Bootstrapper { // Server features. private static final String SERVER_FEATURE_IGNORE_RESOURCE_DELETION = "ignore_resource_deletion"; private static final String SERVER_FEATURE_TRUSTED_XDS_SERVER = "trusted_xds_server"; + private static final String + SERVER_FEATURE_RESOURCE_TIMER_IS_TRANSIENT_ERROR = "resource_timer_is_transient_error"; @VisibleForTesting static boolean enableXdsFallback = GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_FALLBACK, true); + static boolean XdsDataErrorHandlingEnabled + = GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_DATA_ERROR_HANDLING, false); + protected final XdsLogger logger; protected FileReader reader = LocalFileReader.INSTANCE; @@ -247,6 +254,7 @@ private List parseServerInfos(List rawServerConfigs, XdsLogger lo Object implSpecificConfig = getImplSpecificConfig(serverConfig, serverUri); + boolean resourceTimerIsTransientError = false; boolean ignoreResourceDeletion = false; // "For forward compatibility reasons, the client will ignore any entry in the list that it // does not understand, regardless of type." @@ -254,11 +262,14 @@ private List parseServerInfos(List rawServerConfigs, XdsLogger lo if (serverFeatures != null) { logger.log(XdsLogLevel.INFO, "Server features: {0}", serverFeatures); ignoreResourceDeletion = serverFeatures.contains(SERVER_FEATURE_IGNORE_RESOURCE_DELETION); + resourceTimerIsTransientError = XdsDataErrorHandlingEnabled + && serverFeatures.contains(SERVER_FEATURE_RESOURCE_TIMER_IS_TRANSIENT_ERROR); } servers.add( ServerInfo.create(serverUri, implSpecificConfig, ignoreResourceDeletion, serverFeatures != null - && serverFeatures.contains(SERVER_FEATURE_TRUSTED_XDS_SERVER))); + && serverFeatures.contains(SERVER_FEATURE_TRUSTED_XDS_SERVER), + resourceTimerIsTransientError)); } return servers.build(); } diff --git a/xds/src/main/java/io/grpc/xds/client/XdsClient.java b/xds/src/main/java/io/grpc/xds/client/XdsClient.java index 1b53f6778c7..d83725cf5bd 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsClient.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsClient.java @@ -182,6 +182,10 @@ public static ResourceMetadata newResourceMetadataDoesNotExist() { return new ResourceMetadata(ResourceMetadataStatus.DOES_NOT_EXIST, "", 0, false, null, null); } + public static ResourceMetadata newResourceMetadataTimeout() { + return new ResourceMetadata(ResourceMetadataStatus.TIMEOUT, "", 0, false, null, null); + } + public static ResourceMetadata newResourceMetadataAcked( Any rawResource, String version, long updateTimeNanos) { checkNotNull(rawResource, "rawResource"); @@ -239,7 +243,7 @@ public UpdateFailureState getErrorState() { * config_dump.proto */ public enum ResourceMetadataStatus { - UNKNOWN, REQUESTED, DOES_NOT_EXIST, ACKED, NACKED + UNKNOWN, REQUESTED, DOES_NOT_EXIST, ACKED, NACKED, TIMEOUT } /** diff --git a/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java index 034779ed023..d7d1fe408e0 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static io.grpc.xds.client.Bootstrapper.XDSTP_SCHEME; +import static io.grpc.xds.client.BootstrapperImpl.XdsDataErrorHandlingEnabled; import static io.grpc.xds.client.XdsResourceType.ParsedResource; import static io.grpc.xds.client.XdsResourceType.ValidatedResourceUpdate; @@ -69,6 +70,7 @@ public final class XdsClientImpl extends XdsClient implements ResourceStore { // Longest time to wait, since the subscription to some resource, for concluding its absence. @VisibleForTesting public static final int INITIAL_RESOURCE_FETCH_TIMEOUT_SEC = 15; + public static final int EXTENDED_RESOURCE_FETCH_TIMEOUT_SEC = 30; private final SynchronizationContext syncContext = new SynchronizationContext( new Thread.UncaughtExceptionHandler() { @@ -756,6 +758,9 @@ void restartTimer() { // When client becomes ready, it triggers a restartTimer for all relevant subscribers. return; } + ServerInfo serverInfo = activeCpc.getServerInfo(); + int timeoutSec = XdsDataErrorHandlingEnabled && serverInfo.resourceTimerIsTransientError() + ? EXTENDED_RESOURCE_FETCH_TIMEOUT_SEC : INITIAL_RESOURCE_FETCH_TIMEOUT_SEC; class ResourceNotFound implements Runnable { @Override @@ -779,7 +784,7 @@ public String toString() { respTimer.cancel(); } respTimer = syncContext.schedule( - new ResourceNotFound(), INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS, + new ResourceNotFound(), timeoutSec, TimeUnit.SECONDS, timeService); } @@ -858,6 +863,8 @@ void onAbsent(@Nullable ProcessingTracker processingTracker, ServerInfo serverIn // Ignore deletion of State of the World resources when this feature is on, // and the resource is reusable. boolean ignoreResourceDeletionEnabled = serverInfo.ignoreResourceDeletion(); + boolean resourceTimerIsTransientError = + XdsDataErrorHandlingEnabled && serverInfo.resourceTimerIsTransientError(); if (ignoreResourceDeletionEnabled && type.isFullStateOfTheWorld() && data != null) { if (!resourceDeletionIgnored) { logger.log(XdsLogLevel.FORCE_WARNING, @@ -872,13 +879,17 @@ void onAbsent(@Nullable ProcessingTracker processingTracker, ServerInfo serverIn if (!absent) { data = null; absent = true; - metadata = ResourceMetadata.newResourceMetadataDoesNotExist(); + metadata = resourceTimerIsTransientError ? ResourceMetadata.newResourceMetadataTimeout() : + ResourceMetadata.newResourceMetadataDoesNotExist(); for (ResourceWatcher watcher : watchers.keySet()) { if (processingTracker != null) { processingTracker.startTask(); } watchers.get(watcher).execute(() -> { try { + /*This will go after xdsClient watcher APIs are in. + watcher.onResourceChanged(StatusOr.fromStatus(Status.UNAVAILABLE.withDescription( + "Resource " + resource + ": timeout obtaining resource from xDS server")));*/ watcher.onResourceDoesNotExist(resource); } finally { if (processingTracker != null) { diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java index 0ea58c974bb..9e59c3b02f1 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java @@ -3566,7 +3566,7 @@ private static Filter buildHttpConnectionManagerFilter(HttpFilter... httpFilters private XdsResourceType.Args getXdsResourceTypeArgs(boolean isTrustedServer) { return new XdsResourceType.Args( - ServerInfo.create("http://td", "", false, isTrustedServer), "1.0", null, null, null, null + ServerInfo.create("http://td", "", false, isTrustedServer, false), "1.0", null, null, null, null ); } } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index 36131464d08..1759a2d9366 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -348,7 +348,7 @@ public XdsTransport create(ServerInfo serverInfo) { }; xdsServerInfo = ServerInfo.create(SERVER_URI, CHANNEL_CREDENTIALS, ignoreResourceDeletion(), - true); + true, false); BootstrapInfo bootstrapInfo = Bootstrapper.BootstrapInfo.builder() .servers(Collections.singletonList(xdsServerInfo)) @@ -4206,7 +4206,7 @@ private XdsClientImpl createXdsClient(String serverUri) { private BootstrapInfo buildBootStrap(String serverUri) { ServerInfo xdsServerInfo = ServerInfo.create(serverUri, CHANNEL_CREDENTIALS, - ignoreResourceDeletion(), true); + ignoreResourceDeletion(), true, false); return Bootstrapper.BootstrapInfo.builder() .servers(Collections.singletonList(xdsServerInfo)) diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index 622084d4306..f9e81455b0e 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -367,13 +367,13 @@ public void resolving_targetAuthorityInAuthoritiesMap() { String serviceAuthority = "[::FFFF:129.144.52.38]:80"; bootstrapInfo = BootstrapInfo.builder() .servers(ImmutableList.of(ServerInfo.create( - "td.googleapis.com", InsecureChannelCredentials.create(), true, true))) + "td.googleapis.com", InsecureChannelCredentials.create(), true, true, false))) .node(Node.newBuilder().build()) .authorities( ImmutableMap.of(targetAuthority, AuthorityInfo.create( "xdstp://" + targetAuthority + "/envoy.config.listener.v3.Listener/%s?foo=1&bar=2", ImmutableList.of(ServerInfo.create( - "td.googleapis.com", InsecureChannelCredentials.create(), true, true))))) + "td.googleapis.com", InsecureChannelCredentials.create(), true, true, false))))) .build(); expectedLdsResourceName = "xdstp://xds.authority.com/envoy.config.listener.v3.Listener/" + "%5B::FFFF:129.144.52.38%5D:80?bar=2&foo=1"; // query param canonified diff --git a/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java b/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java index 485970741c1..754e903f8a9 100644 --- a/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java +++ b/xds/src/test/java/io/grpc/xds/client/CommonBootstrapperTestUtils.java @@ -203,7 +203,7 @@ public static Bootstrapper.BootstrapInfo buildBootStrap(List serverUris) List serverInfos = new ArrayList<>(); for (String uri : serverUris) { - serverInfos.add(ServerInfo.create(uri, CHANNEL_CREDENTIALS, false, true)); + serverInfos.add(ServerInfo.create(uri, CHANNEL_CREDENTIALS, false, true, false)); } EnvoyProtoData.Node node = EnvoyProtoData.Node.newBuilder().setId("node-id").build();