Copilot commented on code in PR #10473: URL: https://github.com/apache/ozone/pull/10473#discussion_r3383397859
########## hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/ha/TestOMFailoverProxyProviderRefreshWired.java: ########## @@ -0,0 +1,226 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.om.ha; + +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CLIENT_FAILOVER_RESOLVE_NEEDED_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_NODES_KEY; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.net.ConnectException; +import java.net.SocketTimeoutException; +import java.util.StringJoiner; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.io.retry.RetryPolicy; +import org.apache.hadoop.ozone.ha.ConfUtils; +import org.apache.hadoop.ozone.om.exceptions.OMException; +import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; +import org.apache.hadoop.ozone.om.protocolPB.OzoneManagerProtocolPB; +import org.apache.hadoop.security.UserGroupInformation; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Wired-path tests for {@code OMFailoverProxyProviderBase.shouldRetry}'s + * interaction with the new connection-class filter and refresh hook. + * These complement {@code TestConnectionFailureUtils} (helper-in-isolation) + * and {@code TestOMProxyInfoDnsRefresh} (per-instance refresh) by + * exercising the actual retry policy whose return value drives the + * RetryInvocationHandler in production. + * <p> + * The "load-bearing" assertion is that a {@link SocketTimeoutException} + * -- the AWS EC2 / EKS silent-drop case the PR is sold on -- routed + * through {@code shouldRetry} actually triggers the per-node DNS refresh + * on the current OM. {@code TestConnectionFailureUtils} proves the + * filter classifies it correctly in isolation; this test proves the + * filter is wired. + */ +public class TestOMFailoverProxyProviderRefreshWired { + + private static final String OM_SERVICE_ID = "om-svc-refresh-wired"; + private OzoneConfiguration conf; + + @BeforeEach + public void setUp() { + conf = new OzoneConfiguration(); + StringJoiner ids = new StringJoiner(","); + for (int i = 1; i <= 3; i++) { + String nodeId = "om-" + i; + conf.set(ConfUtils.addKeySuffixes(OZONE_OM_ADDRESS_KEY, OM_SERVICE_ID, + nodeId), "localhost:" + (9860 + i)); + ids.add(nodeId); + } + conf.set(ConfUtils.addKeySuffixes(OZONE_OM_NODES_KEY, OM_SERVICE_ID), + ids.toString()); + } + + /** + * A counting subclass that records each call to + * {@code maybeRefreshCurrentOmAddress} so the test can assert + * exactly when the wiring fires. + */ + private static final class CountingProvider + extends HadoopRpcOMFailoverProxyProvider<OzoneManagerProtocolPB> { + int refreshCalls; + + CountingProvider(OzoneConfiguration c) throws IOException { + super(c, UserGroupInformation.getCurrentUser(), OM_SERVICE_ID, + OzoneManagerProtocolPB.class); + } + + @Override + synchronized boolean maybeRefreshCurrentOmAddress() { + refreshCalls++; + return false; + } + } + + /** + * SocketTimeoutException through {@code shouldRetry} -- the AWS + * silent-drop scenario -- must invoke the refresh hook when the + * flag is on. Round 1 personas flagged this exception type as + * missing from the original filter; Round 2 added it to + * ConnectionFailureUtils. This test proves the wiring. + */ + @Test + public void testSocketTimeoutTriggersRefreshHook() throws Exception { + conf.setBoolean(OZONE_CLIENT_FAILOVER_RESOLVE_NEEDED_KEY, true); + CountingProvider p = new CountingProvider(conf); + RetryPolicy policy = p.getRetryPolicy(10); + RetryPolicy.RetryAction action = policy.shouldRetry( + new SocketTimeoutException("EC2 silent drop"), 0, 0, false); + assertEquals(RetryPolicy.RetryAction.RetryDecision.FAILOVER_AND_RETRY, + action.action); + assertEquals(1, p.refreshCalls, + "SocketTimeoutException must invoke the refresh hook exactly once"); + } + + /** + * ConnectException (the OpenStack fast-RST scenario) must also + * invoke the refresh hook. Together with the SocketTimeout test, + * this proves the filter covers both K8s failure shapes the JIRA + * description names. + */ + @Test + public void testConnectExceptionTriggersRefreshHook() throws Exception { + conf.setBoolean(OZONE_CLIENT_FAILOVER_RESOLVE_NEEDED_KEY, true); + CountingProvider p = new CountingProvider(conf); + RetryPolicy policy = p.getRetryPolicy(10); + policy.shouldRetry( + new IOException("connection refused", new ConnectException()), 0, 0, false); + assertEquals(1, p.refreshCalls); Review Comment: This `shouldRetry` invocation exceeds the 120-character Checkstyle limit (tests are included in Checkstyle per the root pom). Please wrap the arguments across lines for readability and to satisfy style checks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
