This is an automated email from the ASF dual-hosted git repository. hulee pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/helix.git
commit b16ac22abd36600039abceab3c5e80956f1dc596 Author: Neal Sun <[email protected]> AuthorDate: Wed Mar 18 10:01:48 2020 -0700 Fix setRoutingData boolean handling; fix leader forwarding url construction (#902) This PR makes SetRoutingData respect the return value of the underlying function; it also fixes request forwarding urls, allowing ports and endpoint prefixes to be added. --- .../accessor/ZkRoutingDataWriter.java | 67 ++++++++++++++++------ .../MetadataStoreDirectoryAccessor.java | 4 +- .../TestZkMetadataStoreDirectory.java | 5 +- .../accessor/TestZkRoutingDataWriter.java | 8 ++- .../MetadataStoreDirectoryAccessorTestBase.java | 5 +- .../rest/server/TestMSDAccessorLeaderElection.java | 12 +++- .../constant/MetadataStoreRoutingConstants.java | 11 +++- 7 files changed, 84 insertions(+), 28 deletions(-) diff --git a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataWriter.java b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataWriter.java index fddd9ee..32b7681 100644 --- a/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataWriter.java +++ b/helix-rest/src/main/java/org/apache/helix/rest/metadatastore/accessor/ZkRoutingDataWriter.java @@ -22,6 +22,7 @@ package org.apache.helix.rest.metadatastore.accessor; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -59,6 +60,10 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter { private static final Logger LOG = LoggerFactory.getLogger(ZkRoutingDataWriter.class); private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private static final String SIMPLE_FIELD_KEY_HOSTNAME = "hostname"; + private static final String SIMPLE_FIELD_KEY_PORT = "port"; + private static final String SIMPLE_FIELD_KEY_CONTEXT_URL_PREFIX = "contextUrlPrefix"; + private final String _namespace; private final HelixZkClient _zkClient; private final ZkDistributedLeaderElection _leaderElection; @@ -89,15 +94,27 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter { String hostName = System.getProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY); if (hostName == null || hostName.isEmpty()) { throw new IllegalStateException( - "Unable to get the hostname of this server instance. System.getProperty fails to fetch " + "Hostname is not set or is empty. System.getProperty fails to fetch " + MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY + "."); } - // remove trailing slash - if (hostName.charAt(hostName.length() - 1) == '/') { - hostName = hostName.substring(0, hostName.length() - 1); - } _myHostName = HttpConstants.HTTP_PROTOCOL_PREFIX + hostName; - ZNRecord myServerInfo = new ZNRecord(_myHostName); + + ZNRecord myServerInfo = new ZNRecord(hostName); + myServerInfo.setSimpleField(SIMPLE_FIELD_KEY_HOSTNAME, hostName); + + String port = System.getProperty(MetadataStoreRoutingConstants.MSDS_SERVER_PORT_KEY); + if (port != null && !port.isEmpty()) { + myServerInfo.setSimpleField(SIMPLE_FIELD_KEY_PORT, port); + } + + // One example of context url prefix is "/admin/v2". With the prefix specified, we want to + // make sure the final url is "/admin/v2/namespaces/NAMESPACE/some/endpoint"; without it + // being specified, we will skip it and go with "/namespaces/NAMESPACE/some/endpoint". + String contextUrlPrefix = + System.getProperty(MetadataStoreRoutingConstants.MSDS_CONTEXT_URL_PREFIX_KEY); + if (contextUrlPrefix != null && !contextUrlPrefix.isEmpty()) { + myServerInfo.setSimpleField(SIMPLE_FIELD_KEY_CONTEXT_URL_PREFIX, contextUrlPrefix); + } _leaderElection = new ZkDistributedLeaderElection(_zkClient, MetadataStoreRoutingConstants.LEADER_ELECTION_ZNODE, myServerInfo); @@ -108,6 +125,22 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter { _forwardHttpClient = HttpClientBuilder.create().setDefaultRequestConfig(config).build(); } + public static String buildEndpointFromLeaderElectionNode(ZNRecord znRecord) { + List<String> urlComponents = + new ArrayList<>(Collections.singletonList(HttpConstants.HTTP_PROTOCOL_PREFIX)); + urlComponents.add(znRecord.getSimpleField(SIMPLE_FIELD_KEY_HOSTNAME)); + String port = znRecord.getSimpleField(SIMPLE_FIELD_KEY_PORT); + if (port != null && !port.isEmpty()) { + urlComponents.add(":"); + urlComponents.add(port); + } + String contextUrlPrefix = znRecord.getSimpleField(SIMPLE_FIELD_KEY_CONTEXT_URL_PREFIX); + if (contextUrlPrefix != null && !contextUrlPrefix.isEmpty()) { + urlComponents.add(contextUrlPrefix); + } + return String.join("", urlComponents); + } + @Override public synchronized boolean addMetadataStoreRealm(String realm) { if (_leaderElection.isLeader()) { @@ -215,9 +248,8 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter { return true; } - String leaderHostName = _leaderElection.getCurrentLeaderInfo().getId(); - String url = leaderHostName + constructUrlSuffix( - MetadataStoreRoutingConstants.MSDS_GET_ALL_ROUTING_DATA_ENDPOINT); + String url = buildEndpointFromLeaderElectionNode(_leaderElection.getCurrentLeaderInfo()) + + constructUrlSuffix(MetadataStoreRoutingConstants.MSDS_GET_ALL_ROUTING_DATA_ENDPOINT); HttpPut httpPut = new HttpPut(url); String routingDataJsonString; try { @@ -231,7 +263,7 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter { return false; } httpPut.setEntity(new StringEntity(routingDataJsonString, ContentType.APPLICATION_JSON)); - return sendRequestToLeader(httpPut, Response.Status.CREATED.getStatusCode(), leaderHostName); + return sendRequestToLeader(httpPut, Response.Status.CREATED.getStatusCode()); } @Override @@ -356,8 +388,8 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter { private boolean buildAndSendRequestToLeader(String urlSuffix, HttpConstants.RestVerbs requestMethod, int expectedResponseCode) throws IllegalArgumentException { - String leaderHostName = _leaderElection.getCurrentLeaderInfo().getId(); - String url = leaderHostName + urlSuffix; + String url = + buildEndpointFromLeaderElectionNode(_leaderElection.getCurrentLeaderInfo()) + urlSuffix; HttpUriRequest request; switch (requestMethod) { case PUT: @@ -370,19 +402,18 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter { throw new IllegalArgumentException("Unsupported requestMethod: " + requestMethod.name()); } - return sendRequestToLeader(request, expectedResponseCode, leaderHostName); + return sendRequestToLeader(request, expectedResponseCode); } // Set to be protected for testing purposes - protected boolean sendRequestToLeader(HttpUriRequest request, int expectedResponseCode, - String leaderHostName) { + protected boolean sendRequestToLeader(HttpUriRequest request, int expectedResponseCode) { try { HttpResponse response = _forwardHttpClient.execute(request); if (response.getStatusLine().getStatusCode() != expectedResponseCode) { HttpEntity respEntity = response.getEntity(); String errorLog = "The forwarded request to leader has failed. Uri: " + request.getURI() + ". Error code: " + response.getStatusLine().getStatusCode() + " Current hostname: " - + _myHostName + " Leader hostname: " + leaderHostName; + + _myHostName; if (respEntity != null) { errorLog += " Response: " + EntityUtils.toString(respEntity); } @@ -391,8 +422,8 @@ public class ZkRoutingDataWriter implements MetadataStoreRoutingDataWriter { } } catch (IOException e) { LOG.error( - "The forwarded request to leader raised an exception. Uri: {} Current hostname: {} Leader hostname: {}", - request.getURI(), _myHostName, leaderHostName, e); + "The forwarded request to leader raised an exception. Uri: {} Current hostname: {} ", + request.getURI(), _myHostName, e); return false; } return true; diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java index f20fd9a..fcaf327 100644 --- a/helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java +++ b/helix-rest/src/main/java/org/apache/helix/rest/server/resources/metadatastore/MetadataStoreDirectoryAccessor.java @@ -235,7 +235,9 @@ public class MetadataStoreDirectoryAccessor extends AbstractResource { Map<String, List<String>> routingData = OBJECT_MAPPER.readValue(jsonContent, new TypeReference<HashMap<String, List<String>>>() { }); - _metadataStoreDirectory.setNamespaceRoutingData(_namespace, routingData); + if (!_metadataStoreDirectory.setNamespaceRoutingData(_namespace, routingData)) { + return serverError(); + } } catch (JsonMappingException | JsonParseException | IllegalArgumentException e) { return badRequest(e.getMessage()); } catch (IOException e) { diff --git a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java index df84754..47d6fba 100644 --- a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java +++ b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/TestZkMetadataStoreDirectory.java @@ -104,7 +104,9 @@ public class TestZkMetadataStoreDirectory extends AbstractTestClass { }); System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY, - getBaseUri().getHost() + ":" + getBaseUri().getPort()); + getBaseUri().getHost()); + System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_PORT_KEY, + Integer.toString(getBaseUri().getPort())); // Create metadataStoreDirectory for (Map.Entry<String, String> entry : _routingZkAddrMap.entrySet()) { @@ -118,6 +120,7 @@ public class TestZkMetadataStoreDirectory extends AbstractTestClass { _metadataStoreDirectory.close(); clearRoutingData(); System.clearProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY); + System.clearProperty(MetadataStoreRoutingConstants.MSDS_SERVER_PORT_KEY); } @Test diff --git a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java index 31db291..217b13e 100644 --- a/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java +++ b/helix-rest/src/test/java/org/apache/helix/rest/metadatastore/accessor/TestZkRoutingDataWriter.java @@ -55,8 +55,7 @@ public class TestZkRoutingDataWriter extends AbstractTestClass { // This method does not call super() because the http call should not be actually made @Override - protected boolean sendRequestToLeader(HttpUriRequest request, int expectedResponseCode, - String leaderHostName) { + protected boolean sendRequestToLeader(HttpUriRequest request, int expectedResponseCode) { calledRequest = request; return false; } @@ -66,7 +65,9 @@ public class TestZkRoutingDataWriter extends AbstractTestClass { public void beforeClass() throws Exception { _zkClient = ZK_SERVER_MAP.get(_zkAddrTestNS).getZkClient(); System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY, - getBaseUri().getHost() + ":" + getBaseUri().getPort()); + getBaseUri().getHost()); + System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_PORT_KEY, + Integer.toString(getBaseUri().getPort())); _zkRoutingDataWriter = new ZkRoutingDataWriter(TEST_NAMESPACE, _zkAddrTestNS); clearRoutingDataPath(); } @@ -74,6 +75,7 @@ public class TestZkRoutingDataWriter extends AbstractTestClass { @AfterClass public void afterClass() throws Exception { System.clearProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY); + System.clearProperty(MetadataStoreRoutingConstants.MSDS_SERVER_PORT_KEY); _zkRoutingDataWriter.close(); clearRoutingDataPath(); } diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/MetadataStoreDirectoryAccessorTestBase.java b/helix-rest/src/test/java/org/apache/helix/rest/server/MetadataStoreDirectoryAccessorTestBase.java index f2ed433..c8c416f 100644 --- a/helix-rest/src/test/java/org/apache/helix/rest/server/MetadataStoreDirectoryAccessorTestBase.java +++ b/helix-rest/src/test/java/org/apache/helix/rest/server/MetadataStoreDirectoryAccessorTestBase.java @@ -96,12 +96,15 @@ public class MetadataStoreDirectoryAccessorTestBase extends AbstractTestClass { _routingDataReader = new ZkRoutingDataReader(TEST_NAMESPACE, _zkAddrTestNS, null); System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY, - getBaseUri().getHost() + ":" + getBaseUri().getPort()); + getBaseUri().getHost()); + System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_PORT_KEY, + Integer.toString(getBaseUri().getPort())); } @AfterClass public void afterClass() throws Exception { System.clearProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY); + System.clearProperty(MetadataStoreRoutingConstants.MSDS_SERVER_PORT_KEY); _routingDataReader.close(); clearRoutingData(); } diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/TestMSDAccessorLeaderElection.java b/helix-rest/src/test/java/org/apache/helix/rest/server/TestMSDAccessorLeaderElection.java index 2a3f0be..b42a101 100644 --- a/helix-rest/src/test/java/org/apache/helix/rest/server/TestMSDAccessorLeaderElection.java +++ b/helix-rest/src/test/java/org/apache/helix/rest/server/TestMSDAccessorLeaderElection.java @@ -36,6 +36,7 @@ import org.apache.helix.rest.common.ContextPropertyKeys; import org.apache.helix.rest.common.HelixRestNamespace; import org.apache.helix.rest.common.HttpConstants; import org.apache.helix.rest.common.ServletType; +import org.apache.helix.rest.metadatastore.accessor.ZkRoutingDataWriter; import org.apache.helix.rest.server.auditlog.AuditLogger; import org.apache.helix.rest.server.filters.CORSFilter; import org.apache.helix.rest.server.mock.MockMetadataStoreDirectoryAccessor; @@ -99,7 +100,9 @@ public class TestMSDAccessorLeaderElection extends MetadataStoreDirectoryAccesso // Set the new uri to be used in leader election System.setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_HOSTNAME_KEY, - getBaseUri().getHost() + ":" + newPort); + getBaseUri().getHost()); + System + .setProperty(MetadataStoreRoutingConstants.MSDS_SERVER_PORT_KEY, Integer.toString(newPort)); // Start http client for testing _httpClient = HttpClients.createDefault(); @@ -217,8 +220,11 @@ public class TestMSDAccessorLeaderElection extends MetadataStoreDirectoryAccesso MetadataStoreRoutingConstants.LEADER_ELECTION_ZNODE + "/" + leaderSelectionNodes.get(0)); ZNRecord secondEphemeralNode = _zkClient.readData( MetadataStoreRoutingConstants.LEADER_ELECTION_ZNODE + "/" + leaderSelectionNodes.get(1)); - Assert.assertEquals(firstEphemeralNode.getId(), _leaderBaseUri); - Assert.assertEquals(secondEphemeralNode.getId(), _mockBaseUri); + Assert.assertEquals(ZkRoutingDataWriter.buildEndpointFromLeaderElectionNode(firstEphemeralNode), + _leaderBaseUri); + Assert + .assertEquals(ZkRoutingDataWriter.buildEndpointFromLeaderElectionNode(secondEphemeralNode), + _mockBaseUri); // Make sure the operation is not done by the follower instance Assert.assertFalse(MockMetadataStoreDirectoryAccessor.operatedOnZk); diff --git a/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/constant/MetadataStoreRoutingConstants.java b/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/constant/MetadataStoreRoutingConstants.java index 766f98a..41d5011 100644 --- a/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/constant/MetadataStoreRoutingConstants.java +++ b/metadata-store-directory-common/src/main/java/org/apache/helix/msdcommon/constant/MetadataStoreRoutingConstants.java @@ -78,7 +78,16 @@ public class MetadataStoreRoutingConstants { // MSDS resource get all sharding keys endpoint string public static final String MSDS_GET_ALL_SHARDING_KEYS_ENDPOINT = "/sharding-keys"; - // The key for system properties that contains the hostname of of the + // The key for system properties that contains the hostname of the // MetadataStoreDirectoryService server instance public static final String MSDS_SERVER_HOSTNAME_KEY = "msds_hostname"; + + // The key for system properties that contains the port of the + // MetadataStoreDirectoryService server instance + public static final String MSDS_SERVER_PORT_KEY = "msds_port"; + + // This is added for helix-rest 2.0. For example, without this value, the url will be + // "localhost:9998"; with this value, the url will be "localhost:9998/admin/v2" if this + // value is "/admin/v2". + public static final String MSDS_CONTEXT_URL_PREFIX_KEY = "msds_context_url_prefix"; }
