MarkGaox commented on code in PR #2588:
URL: https://github.com/apache/helix/pull/2588#discussion_r1293977624
##########
helix-core/src/test/java/org/apache/helix/mock/MockHelixAdmin.java:
##########
@@ -306,6 +305,33 @@ public void enableInstance(String clusterName,
List<String> instances, boolean e
}
+ @Override
+ public void setInstanceOperation(String clusterName, String instanceName,
+ InstanceConstants.InstanceOperation instanceOperation) {
+
+ String path = PropertyPathBuilder.instanceConfig(clusterName,
instanceName);
+
+ if (!_baseDataAccessor.exists(path, 0)) {
+ throw new HelixException(
+ "Cluster " + clusterName + ", instance: " + instanceName + ",
instance config does not exist");
+ }
+
+ _baseDataAccessor.update(path, new DataUpdater<ZNRecord>() {
+ @Override
+ public ZNRecord update(ZNRecord currentData) {
+ if (currentData == null) {
+ throw new HelixException(
+ "Cluster: " + clusterName + ", instance: " + instanceName + ",
participant config is null");
+ }
+
+ InstanceConfig config = new InstanceConfig(currentData);
+ config.setInstanceOperation(instanceOperation); // we set instance
enabled in instance config
+ return config.getRecord();
+ }
+ }, AccessOption.PERSISTENT);
+
Review Comment:
nit: empty line.
##########
helix-rest/src/test/java/org/apache/helix/rest/server/TestPerInstanceAccessor.java:
##########
@@ -486,6 +486,32 @@ public void updateInstance() throws IOException {
Assert.assertEquals(new HashSet<>(instanceConfig.getDisabledPartitionsMap()
.get(CLUSTER_NAME + dbName.substring(0, dbName.length() - 1))),
new HashSet<>(Arrays.asList(CLUSTER_NAME + dbName + "0", CLUSTER_NAME
+ dbName + "3")));
+
+ // test set instance operation
+ new
JerseyUriRequestBuilder("clusters/{}/instances/{}?command=setInstanceOperation&instanceOperation=EVACUATE")
+ .format(CLUSTER_NAME, INSTANCE_NAME).post(this, entity);
+ instanceConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME,
INSTANCE_NAME);
+ Assert.assertEquals(
+ instanceConfig.getInstanceOperation(),
InstanceConstants.InstanceOperation.EVACUATE.toString());
+ // set operation to be DISABLE
+ new
JerseyUriRequestBuilder("clusters/{}/instances/{}?command=setInstanceOperation&instanceOperation=DISABLE")
+ .format(CLUSTER_NAME, INSTANCE_NAME).post(this, entity);
+ instanceConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME,
INSTANCE_NAME);
+ Assert.assertEquals(
+ instanceConfig.getInstanceOperation(),
InstanceConstants.InstanceOperation.DISABLE.toString());
+ Assert.assertTrue(!instanceConfig.getInstanceEnabled());
+ // set operation to EVACUATE, expect error
+ new
JerseyUriRequestBuilder("clusters/{}/instances/{}?command=setInstanceOperation&instanceOperation=EVACUATE")
+ .expectedReturnStatusCode(Response.Status.BAD_REQUEST.getStatusCode())
+ .format(CLUSTER_NAME, INSTANCE_NAME).post(this, entity);
+ // set back to enable
+ new
JerseyUriRequestBuilder("clusters/{}/instances/{}?command=setInstanceOperation&instanceOperation=ENABLE")
+ .format(CLUSTER_NAME, INSTANCE_NAME).post(this, entity);
+ instanceConfig = _configAccessor.getInstanceConfig(CLUSTER_NAME,
INSTANCE_NAME);
+ Assert.assertEquals(
+ instanceConfig.getInstanceOperation(),
InstanceConstants.InstanceOperation.ENABLE.toString());
+ Assert.assertTrue(instanceConfig.getInstanceEnabled());
+
Review Comment:
Suggest to add a case when the `instanceOperation` QueryParam is null. E.x,
"clusters/{}/instances/{}?command=setInstanceOperation"
##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java:
##########
@@ -374,6 +374,43 @@ public void enableInstance(String clusterName,
List<String> instances, boolean e
//enableInstance(clusterName, instances, enabled, null, null);
}
+ @Override
+ // TODO: Name may change in future
+ public void setInstanceOperation(String clusterName, String instanceName,
+ InstanceConstants.InstanceOperation instanceOperation) {
+
+ BaseDataAccessor<ZNRecord> baseAccessor = new
ZkBaseDataAccessor<>(_zkClient);
+ String path = PropertyPathBuilder.instanceConfig(clusterName,
instanceName);
+
+ if (!baseAccessor.exists(path, 0)) {
+ throw new HelixException(
+ "Cluster " + clusterName + ", instance: " + instanceName + ",
instance config does not exist");
+ }
+
+ boolean succeeded = baseAccessor.update(path, new DataUpdater<ZNRecord>() {
Review Comment:
nit: looks like there is an extra space at the front.
--
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]
