You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by hu...@apache.org on 2020/04/08 22:54:15 UTC
[helix] 45/50: Fix setRoutingData boolean handling;
fix leader forwarding url construction (#902)
This is an automated email from the ASF dual-hosted git repository.
hulee pushed a commit to branch zooscalability_merge
in repository https://gitbox.apache.org/repos/asf/helix.git
commit e822c0d738e3625e24c7d7d2a25f5ff7c5bf525f
Author: Neal Sun <ne...@gmail.com>
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";
}