Skip to content

Commit ef09d94

Browse files
ejona86ejona86
authored
Revert "Introduce onResult2 in NameResolver Listener2 that returns Status (#11313)" (#11423)
This reverts commit 9ba2f9d. It causes a channel panic due to unimplemented onResult2(). ``` java.lang.UnsupportedOperationException: Not implemented. at io.grpc.NameResolver$Listener2.onResult2(NameResolver.java:257) at io.grpc.internal.DnsNameResolver$Resolve.lambda$run$0(DnsNameResolver.java:334) at io.grpc.SynchronizationContext.drain(SynchronizationContext.java:94) at io.grpc.SynchronizationContext.execute(SynchronizationContext.java:126) at io.grpc.internal.DnsNameResolver$Resolve.run(DnsNameResolver.java:333) ``` b/356669977
1 parent c37fb18 commit ef09d94

File tree

8 files changed

+168
-501
lines changed

8 files changed

+168
-501
lines changed

‎api/src/main/java/io/grpc/NameResolver.java‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -246,16 +246,6 @@ public final void onAddresses(
246246
*/
247247
@Override
248248
public abstract void onError(Status error);
249-
250-
/**
251-
* Handles updates on resolved addresses and attributes.
252-
*
253-
* @param resolutionResult the resolved server addresses, attributes, and Service Config.
254-
* @since 1.66
255-
*/
256-
public Status onResult2(ResolutionResult resolutionResult) {
257-
throw new UnsupportedOperationException("Not implemented.");
258-
}
259249
}
260250

261251
/**

‎core/src/main/java/io/grpc/internal/DnsNameResolver.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,7 @@ public void run() {
330330
resolutionResultBuilder.setAttributes(result.attributes);
331331
}
332332
}
333-
syncContext.execute(() -> {
334-
savedListener.onResult2(resolutionResultBuilder.build());
335-
});
333+
savedListener.onResult(resolutionResultBuilder.build());
336334
} catch (IOException e) {
337335
savedListener.onError(
338336
Status.UNAVAILABLE.withDescription("Unable to resolve host " + host).withCause(e));

‎core/src/main/java/io/grpc/internal/ManagedChannelImpl.java‎

Lines changed: 127 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -1673,147 +1673,146 @@ final class NameResolverListener extends NameResolver.Listener2 {
16731673
public void onResult(final ResolutionResult resolutionResult) {
16741674
final class NamesResolved implements Runnable {
16751675

1676+
@SuppressWarnings("ReferenceEquality")
16761677
@Override
16771678
public void run() {
1678-
Status status = onResult2(resolutionResult);
1679-
ResolutionResultListener resolutionResultListener = resolutionResult.getAttributes()
1680-
.get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY);
1681-
resolutionResultListener.resolutionAttempted(status);
1682-
}
1683-
}
1684-
1685-
syncContext.execute(new NamesResolved());
1686-
}
1679+
if (ManagedChannelImpl.this.nameResolver != resolver) {
1680+
return;
1681+
}
16871682

1688-
@SuppressWarnings("ReferenceEquality")
1689-
@Override
1690-
public Status onResult2(final ResolutionResult resolutionResult) {
1691-
syncContext.throwIfNotInThisSynchronizationContext();
1692-
if (ManagedChannelImpl.this.nameResolver != resolver) {
1693-
return Status.OK;
1694-
}
1695-
1696-
List<EquivalentAddressGroup> servers = resolutionResult.getAddresses();
1697-
channelLogger.log(
1698-
ChannelLogLevel.DEBUG,
1699-
"Resolved address: {0}, config={1}",
1700-
servers,
1701-
resolutionResult.getAttributes());
1702-
1703-
if (lastResolutionState != ResolutionState.SUCCESS) {
1704-
channelLogger.log(ChannelLogLevel.INFO, "Address resolved: {0}", servers);
1705-
lastResolutionState = ResolutionState.SUCCESS;
1706-
}
1707-
1708-
ConfigOrError configOrError = resolutionResult.getServiceConfig();
1709-
InternalConfigSelector resolvedConfigSelector =
1710-
resolutionResult.getAttributes().get(InternalConfigSelector.KEY);
1711-
ManagedChannelServiceConfig validServiceConfig =
1712-
configOrError != null && configOrError.getConfig() != null
1713-
? (ManagedChannelServiceConfig) configOrError.getConfig()
1714-
: null;
1715-
Status serviceConfigError = configOrError != null ? configOrError.getError() : null;
1716-
1717-
ManagedChannelServiceConfig effectiveServiceConfig;
1718-
if (!lookUpServiceConfig) {
1719-
if (validServiceConfig != null) {
1720-
channelLogger.log(
1721-
ChannelLogLevel.INFO,
1722-
"Service config from name resolver discarded by channel settings");
1723-
}
1724-
effectiveServiceConfig =
1725-
defaultServiceConfig == null ? EMPTY_SERVICE_CONFIG : defaultServiceConfig;
1726-
if (resolvedConfigSelector != null) {
1683+
List<EquivalentAddressGroup> servers = resolutionResult.getAddresses();
17271684
channelLogger.log(
1728-
ChannelLogLevel.INFO,
1729-
"Config selector from name resolver discarded by channel settings");
1730-
}
1731-
realChannel.updateConfigSelector(effectiveServiceConfig.getDefaultConfigSelector());
1732-
} else {
1733-
// Try to use config if returned from name resolver
1734-
// Otherwise, try to use the default config if available
1735-
if (validServiceConfig != null) {
1736-
effectiveServiceConfig = validServiceConfig;
1737-
if (resolvedConfigSelector != null) {
1738-
realChannel.updateConfigSelector(resolvedConfigSelector);
1739-
if (effectiveServiceConfig.getDefaultConfigSelector() != null) {
1685+
ChannelLogLevel.DEBUG,
1686+
"Resolved address: {0}, config={1}",
1687+
servers,
1688+
resolutionResult.getAttributes());
1689+
1690+
if (lastResolutionState != ResolutionState.SUCCESS) {
1691+
channelLogger.log(ChannelLogLevel.INFO, "Address resolved: {0}", servers);
1692+
lastResolutionState = ResolutionState.SUCCESS;
1693+
}
1694+
1695+
ConfigOrError configOrError = resolutionResult.getServiceConfig();
1696+
ResolutionResultListener resolutionResultListener = resolutionResult.getAttributes()
1697+
.get(RetryingNameResolver.RESOLUTION_RESULT_LISTENER_KEY);
1698+
InternalConfigSelector resolvedConfigSelector =
1699+
resolutionResult.getAttributes().get(InternalConfigSelector.KEY);
1700+
ManagedChannelServiceConfig validServiceConfig =
1701+
configOrError != null && configOrError.getConfig() != null
1702+
? (ManagedChannelServiceConfig) configOrError.getConfig()
1703+
: null;
1704+
Status serviceConfigError = configOrError != null ? configOrError.getError() : null;
1705+
1706+
ManagedChannelServiceConfig effectiveServiceConfig;
1707+
if (!lookUpServiceConfig) {
1708+
if (validServiceConfig != null) {
17401709
channelLogger.log(
1741-
ChannelLogLevel.DEBUG,
1742-
"Method configs in service config will be discarded due to presence of"
1743-
+ "config-selector");
1710+
ChannelLogLevel.INFO,
1711+
"Service config from name resolver discarded by channel settings");
1712+
}
1713+
effectiveServiceConfig =
1714+
defaultServiceConfig == null ? EMPTY_SERVICE_CONFIG : defaultServiceConfig;
1715+
if (resolvedConfigSelector != null) {
1716+
channelLogger.log(
1717+
ChannelLogLevel.INFO,
1718+
"Config selector from name resolver discarded by channel settings");
17441719
}
1745-
} else {
17461720
realChannel.updateConfigSelector(effectiveServiceConfig.getDefaultConfigSelector());
1747-
}
1748-
} else if (defaultServiceConfig != null) {
1749-
effectiveServiceConfig = defaultServiceConfig;
1750-
realChannel.updateConfigSelector(effectiveServiceConfig.getDefaultConfigSelector());
1751-
channelLogger.log(
1752-
ChannelLogLevel.INFO,
1753-
"Received no service config, using default service config");
1754-
} else if (serviceConfigError != null) {
1755-
if (!serviceConfigUpdated) {
1756-
// First DNS lookup has invalid service config, and cannot fall back to default
1757-
channelLogger.log(
1758-
ChannelLogLevel.INFO,
1759-
"Fallback to error due to invalid first service config without default config");
1760-
// This error could be an "inappropriate" control plane error that should not bleed
1761-
// through to client code using gRPC. We let them flow through here to the LB as
1762-
// we later check for these error codes when investigating pick results in
1763-
// GrpcUtil.getTransportFromPickResult().
1764-
onError(configOrError.getError());
1765-
return configOrError.getError();
17661721
} else {
1767-
effectiveServiceConfig = lastServiceConfig;
1722+
// Try to use config if returned from name resolver
1723+
// Otherwise, try to use the default config if available
1724+
if (validServiceConfig != null) {
1725+
effectiveServiceConfig = validServiceConfig;
1726+
if (resolvedConfigSelector != null) {
1727+
realChannel.updateConfigSelector(resolvedConfigSelector);
1728+
if (effectiveServiceConfig.getDefaultConfigSelector() != null) {
1729+
channelLogger.log(
1730+
ChannelLogLevel.DEBUG,
1731+
"Method configs in service config will be discarded due to presence of"
1732+
+ "config-selector");
1733+
}
1734+
} else {
1735+
realChannel.updateConfigSelector(effectiveServiceConfig.getDefaultConfigSelector());
1736+
}
1737+
} else if (defaultServiceConfig != null) {
1738+
effectiveServiceConfig = defaultServiceConfig;
1739+
realChannel.updateConfigSelector(effectiveServiceConfig.getDefaultConfigSelector());
1740+
channelLogger.log(
1741+
ChannelLogLevel.INFO,
1742+
"Received no service config, using default service config");
1743+
} else if (serviceConfigError != null) {
1744+
if (!serviceConfigUpdated) {
1745+
// First DNS lookup has invalid service config, and cannot fall back to default
1746+
channelLogger.log(
1747+
ChannelLogLevel.INFO,
1748+
"Fallback to error due to invalid first service config without default config");
1749+
// This error could be an "inappropriate" control plane error that should not bleed
1750+
// through to client code using gRPC. We let them flow through here to the LB as
1751+
// we later check for these error codes when investigating pick results in
1752+
// GrpcUtil.getTransportFromPickResult().
1753+
onError(configOrError.getError());
1754+
if (resolutionResultListener != null) {
1755+
resolutionResultListener.resolutionAttempted(configOrError.getError());
1756+
}
1757+
return;
1758+
} else {
1759+
effectiveServiceConfig = lastServiceConfig;
1760+
}
1761+
} else {
1762+
effectiveServiceConfig = EMPTY_SERVICE_CONFIG;
1763+
realChannel.updateConfigSelector(null);
1764+
}
1765+
if (!effectiveServiceConfig.equals(lastServiceConfig)) {
1766+
channelLogger.log(
1767+
ChannelLogLevel.INFO,
1768+
"Service config changed{0}",
1769+
effectiveServiceConfig == EMPTY_SERVICE_CONFIG ? " to empty" : "");
1770+
lastServiceConfig = effectiveServiceConfig;
1771+
transportProvider.throttle = effectiveServiceConfig.getRetryThrottling();
1772+
}
1773+
1774+
try {
1775+
// TODO(creamsoup): when `servers` is empty and lastResolutionStateCopy == SUCCESS
1776+
// and lbNeedAddress, it shouldn't call the handleServiceConfigUpdate. But,
1777+
// lbNeedAddress is not deterministic
1778+
serviceConfigUpdated = true;
1779+
} catch (RuntimeException re) {
1780+
logger.log(
1781+
Level.WARNING,
1782+
"[" + getLogId() + "] Unexpected exception from parsing service config",
1783+
re);
1784+
}
17681785
}
1769-
} else {
1770-
effectiveServiceConfig = EMPTY_SERVICE_CONFIG;
1771-
realChannel.updateConfigSelector(null);
1772-
}
1773-
if (!effectiveServiceConfig.equals(lastServiceConfig)) {
1774-
channelLogger.log(
1775-
ChannelLogLevel.INFO,
1776-
"Service config changed{0}",
1777-
effectiveServiceConfig == EMPTY_SERVICE_CONFIG ? " to empty" : "");
1778-
lastServiceConfig = effectiveServiceConfig;
1779-
transportProvider.throttle = effectiveServiceConfig.getRetryThrottling();
1780-
}
17811786

1782-
try {
1783-
// TODO(creamsoup): when `servers` is empty and lastResolutionStateCopy == SUCCESS
1784-
// and lbNeedAddress, it shouldn't call the handleServiceConfigUpdate. But,
1785-
// lbNeedAddress is not deterministic
1786-
serviceConfigUpdated = true;
1787-
} catch (RuntimeException re) {
1788-
logger.log(
1789-
Level.WARNING,
1790-
"[" + getLogId() + "] Unexpected exception from parsing service config",
1791-
re);
1787+
Attributes effectiveAttrs = resolutionResult.getAttributes();
1788+
// Call LB only if it's not shutdown. If LB is shutdown, lbHelper won't match.
1789+
if (NameResolverListener.this.helper == ManagedChannelImpl.this.lbHelper) {
1790+
Attributes.Builder attrBuilder =
1791+
effectiveAttrs.toBuilder().discard(InternalConfigSelector.KEY);
1792+
Map<String, ?> healthCheckingConfig =
1793+
effectiveServiceConfig.getHealthCheckingConfig();
1794+
if (healthCheckingConfig != null) {
1795+
attrBuilder
1796+
.set(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG, healthCheckingConfig)
1797+
.build();
1798+
}
1799+
Attributes attributes = attrBuilder.build();
1800+
1801+
Status addressAcceptanceStatus = helper.lb.tryAcceptResolvedAddresses(
1802+
ResolvedAddresses.newBuilder()
1803+
.setAddresses(servers)
1804+
.setAttributes(attributes)
1805+
.setLoadBalancingPolicyConfig(effectiveServiceConfig.getLoadBalancingConfig())
1806+
.build());
1807+
// If a listener is provided, let it know if the addresses were accepted.
1808+
if (resolutionResultListener != null) {
1809+
resolutionResultListener.resolutionAttempted(addressAcceptanceStatus);
1810+
}
1811+
}
17921812
}
17931813
}
17941814

1795-
Attributes effectiveAttrs = resolutionResult.getAttributes();
1796-
// Call LB only if it's not shutdown. If LB is shutdown, lbHelper won't match.
1797-
if (NameResolverListener.this.helper == ManagedChannelImpl.this.lbHelper) {
1798-
Attributes.Builder attrBuilder =
1799-
effectiveAttrs.toBuilder().discard(InternalConfigSelector.KEY);
1800-
Map<String, ?> healthCheckingConfig =
1801-
effectiveServiceConfig.getHealthCheckingConfig();
1802-
if (healthCheckingConfig != null) {
1803-
attrBuilder
1804-
.set(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG, healthCheckingConfig)
1805-
.build();
1806-
}
1807-
Attributes attributes = attrBuilder.build();
1808-
1809-
return helper.lb.tryAcceptResolvedAddresses(
1810-
ResolvedAddresses.newBuilder()
1811-
.setAddresses(servers)
1812-
.setAttributes(attributes)
1813-
.setLoadBalancingPolicyConfig(effectiveServiceConfig.getLoadBalancingConfig())
1814-
.build());
1815-
}
1816-
return Status.OK;
1815+
syncContext.execute(new NamesResolved());
18171816
}
18181817

18191818
@Override

‎core/src/main/java/io/grpc/internal/RetryingNameResolver.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,24 +95,12 @@ public void onResult(ResolutionResult resolutionResult) {
9595
"RetryingNameResolver can only be used once to wrap a NameResolver");
9696
}
9797

98-
// To have retry behavior for name resolvers that haven't migrated to onResult2.
9998
delegateListener.onResult(resolutionResult.toBuilder().setAttributes(
10099
resolutionResult.getAttributes().toBuilder()
101100
.set(RESOLUTION_RESULT_LISTENER_KEY, new ResolutionResultListener()).build())
102101
.build());
103102
}
104103

105-
@Override
106-
public Status onResult2(ResolutionResult resolutionResult) {
107-
Status status = delegateListener.onResult2(resolutionResult);
108-
if (status.isOk()) {
109-
retryScheduler.reset();
110-
} else {
111-
retryScheduler.schedule(new DelayedNameResolverRefresh());
112-
}
113-
return status;
114-
}
115-
116104
@Override
117105
public void onError(Status error) {
118106
delegateListener.onError(error);

0 commit comments

Comments
 (0)