adoroszlai commented on code in PR #4133:
URL: https://github.com/apache/ozone/pull/4133#discussion_r1060355923
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -444,6 +450,13 @@ private enum State {
private final boolean isSecurityEnabled;
+
+ /** A list of property that are reconfigurable at runtime. */
+ private final TreeSet<String> reconfigurableProperties = Sets.newTreeSet(
+ Lists.newArrayList(
+ OZONE_ADMINISTRATORS
+ ));
Review Comment:
Please use `ImmutableSortedSet` instead. Also, please declare as
`SortedSet` instead of the implementation.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/ReconfigOMSubcommand.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.admin.om;
+
+import org.apache.hadoop.conf.ReconfigurationTaskStatus;
+import org.apache.hadoop.conf.ReconfigurationUtil;
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.net.NetUtils;
+import org.apache.hadoop.ozone.om.helpers.OMNodeDetails;
+import org.apache.hadoop.ozone.om.protocolPB.OMAdminProtocolClientSideImpl;
+import org.apache.hadoop.security.UserGroupInformation;
+import picocli.CommandLine;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.Callable;
+
+/**
+ * Handler of om roles command.
+ */
[email protected](
+ name = "reconfig",
+ customSynopsis = "ozone admin om reconfig -address=<ip:port> -op=" +
+ "start|status|properties",
+ description = "Dynamic reconfiguring without stopping the OM.\n" +
+ "Three operations are provided:\n" +
+ "\tstart: Execute the reconfig operation asynchronously\n" +
+ "\tstatus: Check reconfiguration status\n" +
+ "\tproperties: List reconfigurable properties",
+ mixinStandardHelpOptions = true,
+ versionProvider = HddsVersionProvider.class)
+public class ReconfigOMSubcommand implements Callable<Void> {
+
+ @CommandLine.ParentCommand
+ private OMAdmin parent;
+
+ @CommandLine.Option(names = {"-address", "--host-port"},
Review Comment:
Please avoid adding long options with single dash.
```suggestion
@CommandLine.Option(names = {"--address"},
```
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/ReconfigOMSubcommand.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * 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.admin.om;
+
+import org.apache.hadoop.conf.ReconfigurationTaskStatus;
+import org.apache.hadoop.conf.ReconfigurationUtil;
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.net.NetUtils;
+import org.apache.hadoop.ozone.om.helpers.OMNodeDetails;
+import org.apache.hadoop.ozone.om.protocolPB.OMAdminProtocolClientSideImpl;
+import org.apache.hadoop.security.UserGroupInformation;
+import picocli.CommandLine;
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.Callable;
+
+/**
+ * Handler of om roles command.
+ */
[email protected](
+ name = "reconfig",
+ customSynopsis = "ozone admin om reconfig -address=<ip:port> -op=" +
+ "start|status|properties",
+ description = "Dynamic reconfiguring without stopping the OM.\n" +
+ "Three operations are provided:\n" +
+ "\tstart: Execute the reconfig operation asynchronously\n" +
+ "\tstatus: Check reconfiguration status\n" +
+ "\tproperties: List reconfigurable properties",
+ mixinStandardHelpOptions = true,
+ versionProvider = HddsVersionProvider.class)
+public class ReconfigOMSubcommand implements Callable<Void> {
+
+ @CommandLine.ParentCommand
+ private OMAdmin parent;
+
+ @CommandLine.Option(names = {"-address", "--host-port"},
+ description = "om address: <ip:port> or <hostname:port>",
+ required = true)
+ private String address;
+
+ @CommandLine.Option(names = {"-op"},
+ description = "op",
+ required = true)
+ private String op;
+
+ private OzoneConfiguration ozoneConf;
+ private UserGroupInformation user;
+
+ @Override
+ public Void call() throws IOException {
+ ozoneConf = new OzoneConfiguration();
+ user = UserGroupInformation.getCurrentUser();
+
+ InetSocketAddress socketAddr = NetUtils.createSocketAddr(address);
+ OMNodeDetails omNode = new
OMNodeDetails.Builder().setRpcAddress(socketAddr)
+ .build();
+ try (OMAdminProtocolClientSideImpl omAdminProtocolClient =
+ OMAdminProtocolClientSideImpl.createProxyForSingleOM(ozoneConf,
+ user, omNode)) {
+
+ if ("start".equals(op)) {
+ omAdminProtocolClient.startOmReconfiguration();
+ System.out.printf("Started OM reconfiguration task on node [%s].%n",
+ address);
+
+ } else if ("status".equals(op)) {
+ ReconfigurationTaskStatus status =
+ omAdminProtocolClient.getOmReconfigurationStatus();
+ System.out.printf("Reconfiguring status for node [%s]: ", address);
+ printReconfigurationStatus(status);
+
+ } else if ("properties".equals(op)) {
+ List<String> properties =
+ omAdminProtocolClient.listOmReconfigurableProperties();
+ System.out.printf("OM Node [%s] Reconfigurable properties:%n",
address);
+ for (String name : properties) {
+ System.out.println(name);
+ }
+ } else {
+ System.err.println("Unknown operation: " + op);
+ }
Review Comment:
Please use picocli builtin support for separate subcommands instead of
implementing a custom way to specify operations. See `SafeModeCommands` for
example.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocol/OMAdminProtocol.java:
##########
@@ -39,4 +43,23 @@ public interface OMAdminProtocol extends Closeable {
* Remove OM from HA ring.
*/
void decommission(OMNodeDetails removeOMNode) throws IOException;
+
+ /**
+ * Asynchronously reload configuration on disk and apply changes.
+ */
+ @Idempotent
+ void startOmReconfiguration() throws IOException;
+
+ /**
+ * Get the status of the previously issued reconfig task.
+ * @see org.apache.hadoop.conf.ReconfigurationTaskStatus
+ */
+ @Idempotent
+ ReconfigurationTaskStatus getOmReconfigurationStatus() throws IOException;
+
+ /**
+ * Get a list of allowed properties for reconfiguration.
+ */
+ @Idempotent
+ List<String> listOmReconfigurableProperties() throws IOException;
Review Comment:
Please define these in a separate interface (without reference to `Om`) and
let `OMAdminProtocol` extend that interface.
##########
hadoop-ozone/interface-client/src/main/proto/OMAdminProtocol.proto:
##########
@@ -69,16 +69,58 @@ message DecommissionOMResponse {
optional string errorMsg = 3;
}
+/** Asks OM to reload configuration file. */
+message StartOmReconfigurationRequestProto {
+}
+
+message StartOmReconfigurationResponseProto {
+}
+
+/** Query the running status of reconfiguration process */
+message GetOmReconfigurationStatusRequestProto {
+}
+
+message GetOmReconfigurationStatusConfigChangeProto {
+ required string name = 1;
+ required string oldValue = 2;
+ optional string newValue = 3;
+ optional string errorMessage = 4; // It is empty if success.
+}
+
+message GetOmReconfigurationStatusResponseProto {
+ required int64 startTime = 1;
+ optional int64 endTime = 2;
+ repeated GetOmReconfigurationStatusConfigChangeProto changes = 3;
+}
+
+/** Query the reconfigurable properties on OM. */
+message ListOmReconfigurablePropertiesRequestProto {
+}
+
+message ListOmReconfigurablePropertiesResponseProto {
+ repeated string name = 1;
+}
Review Comment:
I think these messages should be defined in a service-agnostic way (in
`hdds.proto` and without references to `Om`) to avoid duplication if we would
like to implement similar feature in SCM and other services.
--
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]
