This is an automated email from the ASF dual-hosted git repository.
xyuanlu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git
The following commit(s) were added to refs/heads/master by this push:
new 8c40bd9c0 Fix stoppable parallel instances API swallowing error and
add proper error handling for RESTConfig getBaseUrl method(asserts may not
throw AssertionException if assertEnabled is specifically enabled in jvm
config). Remove assumptioin that instance name contains port information while
maintaining backwards compatibility. (#2575)
8c40bd9c0 is described below
commit 8c40bd9c066a9b6643f793ca9389aeafcf005f7d
Author: Zachary Pinto <[email protected]>
AuthorDate: Wed Jul 26 17:31:52 2023 -0700
Fix stoppable parallel instances API swallowing error and add proper error
handling for RESTConfig getBaseUrl method(asserts may not throw
AssertionException if assertEnabled is specifically enabled in jvm config).
Remove assumptioin that instance name contains port information while
maintaining backwards compatibility. (#2575)
Fix stoppable parallel instances API swallowing error thrown when
processing stoppable checks, add proper error handling for RESTConfig
getBaseUrl method(asserts may not throw AssertionException if assertEnabled is
specifically enabled in jvm config), and remove assumption that instance name
contains port information while maintaining backwards compatibility.
---
.../java/org/apache/helix/model/RESTConfig.java | 16 ++++++--
.../org/apache/helix/model/TestRESTConfig.java | 46 ++++++++++++++++++++++
.../MaintenanceManagementService.java | 8 ++--
3 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/helix-core/src/main/java/org/apache/helix/model/RESTConfig.java
b/helix-core/src/main/java/org/apache/helix/model/RESTConfig.java
index aff56b9af..90b1c85cf 100644
--- a/helix-core/src/main/java/org/apache/helix/model/RESTConfig.java
+++ b/helix-core/src/main/java/org/apache/helix/model/RESTConfig.java
@@ -19,6 +19,7 @@ package org.apache.helix.model;
* under the License.
*/
+import org.apache.helix.HelixException;
import org.apache.helix.HelixProperty;
import org.apache.helix.zookeeper.datamodel.ZNRecord;
@@ -67,12 +68,19 @@ public class RESTConfig extends HelixProperty {
*/
public String getBaseUrl(String instance) {
String baseUrl = get(RESTConfig.SimpleFields.CUSTOMIZED_HEALTH_URL);
+
// pre-assumption of the url, must be format of "http://*/path";, the
wildcard is replaceable by
// the instance vip
- assert baseUrl.contains("*");
- // pre-assumption of the instance name, must be format of
<instanceVip>_<port>
- assert instance.contains("_");
- String instanceVip = instance.substring(0, instance.indexOf('_'));
+ if (baseUrl == null || !baseUrl.contains("*")) {
+ throw new HelixException("Invalid CUSTOMIZED_HEALTH_URL in REST config:
" + baseUrl);
+ }
+
+ String instanceVip = instance;
+ // instance name format could be <instanceVip>_<port> so we need to
extract the instance vip
+ if (instance.contains("_")) {
+ instanceVip = instance.substring(0, instance.indexOf('_'));
+ }
+
return baseUrl.replace("*", instanceVip);
}
}
diff --git
a/helix-core/src/test/java/org/apache/helix/model/TestRESTConfig.java
b/helix-core/src/test/java/org/apache/helix/model/TestRESTConfig.java
new file mode 100644
index 000000000..fb93b90af
--- /dev/null
+++ b/helix-core/src/test/java/org/apache/helix/model/TestRESTConfig.java
@@ -0,0 +1,46 @@
+package org.apache.helix.model;
+
+/*
+ * 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.
+ */
+
+import org.apache.helix.HelixException;
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class TestRESTConfig {
+ @Test
+ public void testGetBaseUrlValid() {
+ ZNRecord record = new ZNRecord("test");
+
record.setSimpleField(RESTConfig.SimpleFields.CUSTOMIZED_HEALTH_URL.name(),
"http://*:8080";);
+ RESTConfig restConfig = new RESTConfig(record);
+ Assert.assertEquals(restConfig.getBaseUrl("instance0"),
"http://instance0:8080";);
+ Assert.assertEquals(restConfig.getBaseUrl("instance1_9090"),
"http://instance1:8080";);
+ }
+
+ @Test(expectedExceptions = HelixException.class)
+ public void testGetBaseUrlInvalid() {
+ ZNRecord record = new ZNRecord("test");
+
record.setSimpleField(RESTConfig.SimpleFields.CUSTOMIZED_HEALTH_URL.name(),
"http://foo:8080";);
+ RESTConfig restConfig = new RESTConfig(record);
+ restConfig.getBaseUrl("instance0");
+ }
+}
diff --git
a/helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java
b/helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java
index 440898b62..657619c9e 100644
---
a/helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java
+++
b/helix-rest/src/main/java/org/apache/helix/rest/clusterMaintenanceService/MaintenanceManagementService.java
@@ -28,7 +28,6 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
@@ -538,8 +537,11 @@ public class MaintenanceManagementService {
// will be checked in the next round
instancesForNextCheck.add(instance);
}
- } catch (InterruptedException | ExecutionException e) {
- LOG.error("Failed to get StoppableChecks in parallel. Instance: {}",
instance, e);
+ } catch (Exception e) {
+ String errorMessage =
+ String.format("Failed to get StoppableChecks in parallel.
Instance: %s", instance);
+ LOG.error(errorMessage, e);
+ throw new HelixException(errorMessage);
}
}
