You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/04/17 06:46:14 UTC
[GitHub] [incubator-pinot] fx19880617 opened a new pull request #5266: Add
PinotServiceManager to start Pinot components
fx19880617 opened a new pull request #5266: Add PinotServiceManager to start Pinot components
URL: https://github.com/apache/incubator-pinot/pull/5266
1. Defines Enum ServiceRole: CONTROLLER/BROKER/SERVER/MINION for the Pinot Components to manage
2. Defines interfaces: ServiceStartable, so each role could implement its own way to start/stop it.
3. Implement PinotServiceManagerStarter as a new entry point to manage all pinot roles lifecycles/
4. Make ServiceStatus could handle multiple Pinot roles.
5. Provide REST API to start/stop Pinot Roles with default configs.
6. Move ControllerStarter/BrokerStarter/ServerStarter commands to use new ServiceManagerStarer command.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#discussion_r429123256
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStatus.java
##########
@@ -43,17 +44,79 @@
*/
@SuppressWarnings("unused")
public class ServiceStatus {
+ public static final String STATUS_DESCRIPTION_NONE = "None";
+ public static final String STATUS_DESCRIPTION_INIT = "Init";
+ public static final String STATUS_DESCRIPTION_STARTED = "Started";
+ public static final String STATUS_DESCRIPTION_NO_HELIX_STATE = "Helix state does not exist";
private static final Logger LOGGER = LoggerFactory.getLogger(ServiceStatus.class);
+ private static final int MAX_RESOURCE_NAMES_TO_LOG = 5;
+ private static final Map<String, ServiceStatusCallback> serviceStatusCallbackMap = new ConcurrentHashMap<>();
+ private static final ServiceStatusCallback serviceStatusCallback =
+ new MapBasedMultipleCallbackServiceStatusCallback(serviceStatusCallbackMap);
- public enum Status {
- STARTING, GOOD, BAD
+ public static void setServiceStatusCallback(String name, ServiceStatusCallback serviceStatusCallback) {
+ ServiceStatus.serviceStatusCallbackMap.put(name, serviceStatusCallback);
}
- public static final String STATUS_DESCRIPTION_NONE = "None";
- public static final String STATUS_DESCRIPTION_INIT = "Init";
- public static final String STATUS_DESCRIPTION_NO_HELIX_STATE = "Helix state does not exist";
+ public static void removeServiceStatusCallback(String name) {
+ ServiceStatus.serviceStatusCallbackMap.remove(name);
+ }
- private static final int MAX_RESOURCE_NAMES_TO_LOG = 5;
+ public static Status getServiceStatus() {
+ return getServiceStatus(serviceStatusCallback);
+ }
+
+ public static Status getServiceStatus(String name) {
+ if (serviceStatusCallbackMap.containsKey(name)) {
+ return getServiceStatus(serviceStatusCallbackMap.get(name));
+ } else {
+ return Status.NOT_STARTED;
Review comment:
We have status of `NOT_STARTED`, `STARTING`, `GOOD`, `BAD` for now.
I feel it's ok to say `NOT_STARTED` after we stop an instance, as it's still possible to start a new instance later on.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#issuecomment-632925013
> can we list the commands needed to start Pinot after this gets in?
See PR description.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#discussion_r421119352
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/service/api/resources/PinotServiceManagerInstanceResource.java
##########
@@ -0,0 +1,241 @@
+/**
+ * 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.pinot.tools.service.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.NetUtil;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Controller.CONFIG_OF_CONTROLLER_METRICS_PREFIX;
Review comment:
remove static imports?
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
##########
@@ -0,0 +1,213 @@
+/**
+ * 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.pinot.tools.admin.command;
+
+import java.io.File;
+import java.net.SocketException;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.commons.configuration.Configuration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.tools.Command;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.apache.pinot.tools.utils.PinotConfigUtils;
+import org.kohsuke.args4j.Option;
+import org.kohsuke.args4j.spi.StringArrayOptionHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Helix.PINOT_SERVICE_ROLE;
+
+
+/**
+ * Class to implement StartPinotService command.
+ *
+ */
+public class StartServiceManagerCommand extends AbstractBaseAdminCommand implements Command {
Review comment:
Could you add an example in the description of how to run this command
1. When providing multiple bootstrapConfigPath
2. When starting multiple roles with default configs
Or if there's any documentation, having the link here also works
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
##########
@@ -0,0 +1,213 @@
+/**
+ * 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.pinot.tools.admin.command;
+
+import java.io.File;
+import java.net.SocketException;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.commons.configuration.Configuration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.tools.Command;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.apache.pinot.tools.utils.PinotConfigUtils;
+import org.kohsuke.args4j.Option;
+import org.kohsuke.args4j.spi.StringArrayOptionHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Helix.PINOT_SERVICE_ROLE;
+
+
+/**
+ * Class to implement StartPinotService command.
+ *
+ */
+public class StartServiceManagerCommand extends AbstractBaseAdminCommand implements Command {
+ private static final Logger LOGGER = LoggerFactory.getLogger(StartServiceManagerCommand.class);
+ private final List<Configuration> _bootstrapConfigurations = new ArrayList<>();
+ private final String[] BOOTSTRAP_SERVICES = new String[]{"CONTROLLER", "BROKER", "SERVER"};
+
+ @Option(name = "-help", required = false, help = true, aliases = {"-h", "--h", "--help"}, usage = "Print this message.")
+ private boolean _help;
+ @Option(name = "-zkAddress", required = true, metaVar = "<http>", usage = "Http address of Zookeeper.")
+ private String _zkAddress = DEFAULT_ZK_ADDRESS;
+ @Option(name = "-clusterName", required = true, metaVar = "<String>", usage = "Pinot cluster name.")
+ private String _clusterName = DEFAULT_CLUSTER_NAME;
+ @Option(name = "-port", required = true, metaVar = "<int>", usage = "Pinot service manager admin port, -1 means disable, 0 means a random available port.")
+ private int _port;
+ @Option(name = "-bootstrapConfigPaths", handler = StringArrayOptionHandler.class, required = false, usage = "A list of Pinot service config file paths. Each config file requires an extra config: 'pinot.service.role' to indicate which service to start.", forbids = {"-bootstrapServices"})
+ private String[] _bootstrapConfigPaths;
+ @Option(name = "-bootstrapServices", handler = StringArrayOptionHandler.class, required = false, usage = "A list of Pinot service roles to start with default config. E.g. CONTROLLER/BROKER/SERVER", forbids = {"-bootstrapConfigPaths"})
+ private String[] _bootstrapServices = BOOTSTRAP_SERVICES;
+
+ private PinotServiceManager _pinotServiceManager;
+
+ public String getZkAddress() {
+ return _zkAddress;
+ }
+
+ public StartServiceManagerCommand setZkAddress(String zkAddress) {
+ _zkAddress = zkAddress;
+ return this;
+ }
+
+ public String getClusterName() {
+ return _clusterName;
+ }
+
+ public StartServiceManagerCommand setClusterName(String clusterName) {
+ _clusterName = clusterName;
+ return this;
+ }
+
+ public int getPort() {
+ return _port;
+ }
+
+ public StartServiceManagerCommand setPort(int port) {
+ _port = port;
+ return this;
+ }
+
+ public String[] getBootstrapConfigPaths() {
+ return _bootstrapConfigPaths;
+ }
+
+ public StartServiceManagerCommand setBootstrapConfigPaths(String[] bootstrapConfigPaths) {
+ _bootstrapConfigPaths = bootstrapConfigPaths;
+ return this;
+ }
+
+ public String[] getBootstrapServices() {
+ return _bootstrapServices;
+ }
+
+ public StartServiceManagerCommand setBootstrapServices(String[] bootstrapServices) {
+ _bootstrapServices = bootstrapServices;
+ return this;
+ }
+
+ @Override
+ public boolean getHelp() {
+ return _help;
+ }
+
+ public void setHelp(boolean help) {
+ _help = help;
+ }
+
+ @Override
+ public String getName() {
+ return "StartPinotService";
+ }
+
+ @Override
+ public String toString() {
+ StringBuilder sb = new StringBuilder()
+ .append("StartServiceManager -clusterName " + _clusterName + " -zkAddress " + _zkAddress + " -port " + _port);
+ if (_bootstrapConfigPaths != null) {
+ sb.append(" -bootstrapConfigPaths " + Arrays.toString(_bootstrapConfigPaths));
+ } else if (_bootstrapServices != null) {
+ sb.append(" -bootstrapServices " + Arrays.toString(_bootstrapServices));
+ }
+
+ return sb.toString();
+ }
+
+ @Override
+ public void cleanup() {
+ }
+
+ @Override
+ public String description() {
+ return "Start the Pinot Service Process at the specified port.";
Review comment:
this description is not very clear. "Starts the PinotServiceManager..." ?
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/service/PinotServiceManagerAdminApiApplication.java
##########
@@ -0,0 +1,89 @@
+/**
+ * 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.pinot.tools.service;
+
+import com.google.common.base.Preconditions;
+import io.swagger.jaxrs.config.BeanConfig;
+import java.net.URI;
+import java.net.URL;
+import java.net.URLClassLoader;
+import org.glassfish.grizzly.http.server.CLStaticHttpHandler;
+import org.glassfish.grizzly.http.server.HttpHandler;
+import org.glassfish.grizzly.http.server.HttpServer;
+import org.glassfish.hk2.utilities.binding.AbstractBinder;
+import org.glassfish.jersey.grizzly2.httpserver.GrizzlyHttpServerFactory;
+import org.glassfish.jersey.jackson.JacksonFeature;
+import org.glassfish.jersey.server.ResourceConfig;
+
+
Review comment:
Some javadoc?
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/EnumArrayOptionHandler.java
##########
@@ -0,0 +1,67 @@
+/**
+ * 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.pinot.tools.admin.command;
+
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.OptionDef;
+import org.kohsuke.args4j.spi.OptionHandler;
+import org.kohsuke.args4j.spi.Parameters;
+import org.kohsuke.args4j.spi.Setter;
+
+
+public class EnumArrayOptionHandler<T extends Enum<T>> extends OptionHandler<T> {
+
+ public EnumArrayOptionHandler(CmdLineParser parser, OptionDef option, Setter<T> setter) {
+ super(parser, option, setter);
+ }
+
+ /**
+ * Returns {@code "T[]"}.
+ *
+ * @return return "T[]";
+ */
+ @Override
+ public String getDefaultMetaVariable() {
+ return setter.getType().getName() + "[]";
+ }
+
+ /**
+ * Tries to parse {@code String[]} argument from {@link Parameters}.
+ */
+ @Override
+ public int parseArguments(Parameters params)
+ throws CmdLineException {
+ int counter = 0;
+ for (; counter < params.size(); counter++) {
+ String param = params.getParameter(counter);
+
+ if (param.startsWith("-")) {
+ break;
+ }
+
+ for (String p : param.split(" ")) {
+ Class<T> t = (Class<T>) setter.getType();
+ setter.addValue(Enum.valueOf(t, p));
+ }
+ }
+
+ return counter;
+ }
+}
Review comment:
new line
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/EnumArrayOptionHandler.java
##########
@@ -0,0 +1,67 @@
+/**
+ * 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.pinot.tools.admin.command;
+
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.OptionDef;
+import org.kohsuke.args4j.spi.OptionHandler;
+import org.kohsuke.args4j.spi.Parameters;
+import org.kohsuke.args4j.spi.Setter;
+
+
+public class EnumArrayOptionHandler<T extends Enum<T>> extends OptionHandler<T> {
Review comment:
Some javadoc for this?
Also, where is it used?
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/service/api/resources/PinotServiceManagerInstanceResource.java
##########
@@ -0,0 +1,241 @@
+/**
+ * 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.pinot.tools.service.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.NetUtil;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Controller.CONFIG_OF_CONTROLLER_METRICS_PREFIX;
+import static org.apache.pinot.common.utils.CommonConstants.Controller.DEFAULT_METRICS_PREFIX;
+import static org.apache.pinot.tools.utils.PinotConfigUtils.TMP_DIR;
+import static org.apache.pinot.tools.utils.PinotConfigUtils.getAvailablePort;
+
+
+@Api(tags = "Startable")
+@Path("/")
+public class PinotServiceManagerInstanceResource {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(PinotServiceManagerInstanceResource.class);
+
+ @Inject
+ private PinotServiceManager _pinotServiceManager;
+
+ @GET
+ @Produces(MediaType.APPLICATION_JSON)
+ @Path("/instances")
+ @ApiOperation(value = "Get Pinot Instances Status")
+ @ApiResponses(value = {@ApiResponse(code = 200, message = "Instance Status"), @ApiResponse(code = 500, message = "Internal server error")})
+ public Map<String, PinotInstanceStatus> getPinotAllInstancesStatus() {
+ Map<String, PinotInstanceStatus> results = new HashMap<>();
+ for (String instanceId : _pinotServiceManager.getRunningInstanceIds()) {
+ results.put(instanceId, _pinotServiceManager.getInstanceStatus(instanceId));
+ }
+ return results;
+ }
+
+ @GET
+ @Produces(MediaType.APPLICATION_JSON)
+ @Path("/instances/{instanceName}")
+ @ApiOperation(value = "Get Pinot Instance Status")
+ @ApiResponses(value = {@ApiResponse(code = 200, message = "Instance Status"), @ApiResponse(code = 404, message = "Instance Not Found"), @ApiResponse(code = 500, message = "Internal server error")})
+ public PinotInstanceStatus getPinotInstanceStatus(
+ @ApiParam(value = "Name of the instance") @PathParam("instanceName") String instanceName) {
+ List<String> instanceIds = _pinotServiceManager.getRunningInstanceIds();
+ if (instanceIds.contains(instanceName)) {
+ return _pinotServiceManager.getInstanceStatus(instanceName);
+ }
+ throw new WebApplicationException(String.format("Instance [%s] not found.", instanceName),
+ Response.Status.NOT_FOUND);
+ }
+
+ @DELETE
Review comment:
is it possible to add "stopAllServers" "stopAllBrokers" etc, and "stopAll"
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
##########
@@ -0,0 +1,213 @@
+/**
+ * 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.pinot.tools.admin.command;
+
+import java.io.File;
+import java.net.SocketException;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.commons.configuration.Configuration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.tools.Command;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.apache.pinot.tools.utils.PinotConfigUtils;
+import org.kohsuke.args4j.Option;
+import org.kohsuke.args4j.spi.StringArrayOptionHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Helix.PINOT_SERVICE_ROLE;
+
+
+/**
+ * Class to implement StartPinotService command.
+ *
+ */
+public class StartServiceManagerCommand extends AbstractBaseAdminCommand implements Command {
+ private static final Logger LOGGER = LoggerFactory.getLogger(StartServiceManagerCommand.class);
+ private final List<Configuration> _bootstrapConfigurations = new ArrayList<>();
+ private final String[] BOOTSTRAP_SERVICES = new String[]{"CONTROLLER", "BROKER", "SERVER"};
+
+ @Option(name = "-help", required = false, help = true, aliases = {"-h", "--h", "--help"}, usage = "Print this message.")
+ private boolean _help;
+ @Option(name = "-zkAddress", required = true, metaVar = "<http>", usage = "Http address of Zookeeper.")
+ private String _zkAddress = DEFAULT_ZK_ADDRESS;
+ @Option(name = "-clusterName", required = true, metaVar = "<String>", usage = "Pinot cluster name.")
+ private String _clusterName = DEFAULT_CLUSTER_NAME;
+ @Option(name = "-port", required = true, metaVar = "<int>", usage = "Pinot service manager admin port, -1 means disable, 0 means a random available port.")
Review comment:
I couldn't find the logic which handles port=-1.
I see port being set as -1 in StartControllerCommnad, StartBrokerCommand etc. But in PinotServiceManager I didn't find any special handling for it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#discussion_r429119690
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/service/api/resources/PinotServiceManagerInstanceResource.java
##########
@@ -0,0 +1,241 @@
+/**
+ * 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.pinot.tools.service.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.NetUtil;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Controller.CONFIG_OF_CONTROLLER_METRICS_PREFIX;
+import static org.apache.pinot.common.utils.CommonConstants.Controller.DEFAULT_METRICS_PREFIX;
+import static org.apache.pinot.tools.utils.PinotConfigUtils.TMP_DIR;
+import static org.apache.pinot.tools.utils.PinotConfigUtils.getAvailablePort;
+
+
+@Api(tags = "Startable")
+@Path("/")
+public class PinotServiceManagerInstanceResource {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(PinotServiceManagerInstanceResource.class);
+
+ @Inject
+ private PinotServiceManager _pinotServiceManager;
+
+ @GET
+ @Produces(MediaType.APPLICATION_JSON)
+ @Path("/instances")
+ @ApiOperation(value = "Get Pinot Instances Status")
+ @ApiResponses(value = {@ApiResponse(code = 200, message = "Instance Status"), @ApiResponse(code = 500, message = "Internal server error")})
+ public Map<String, PinotInstanceStatus> getPinotAllInstancesStatus() {
+ Map<String, PinotInstanceStatus> results = new HashMap<>();
+ for (String instanceId : _pinotServiceManager.getRunningInstanceIds()) {
+ results.put(instanceId, _pinotServiceManager.getInstanceStatus(instanceId));
+ }
+ return results;
+ }
+
+ @GET
+ @Produces(MediaType.APPLICATION_JSON)
+ @Path("/instances/{instanceName}")
+ @ApiOperation(value = "Get Pinot Instance Status")
+ @ApiResponses(value = {@ApiResponse(code = 200, message = "Instance Status"), @ApiResponse(code = 404, message = "Instance Not Found"), @ApiResponse(code = 500, message = "Internal server error")})
+ public PinotInstanceStatus getPinotInstanceStatus(
+ @ApiParam(value = "Name of the instance") @PathParam("instanceName") String instanceName) {
+ List<String> instanceIds = _pinotServiceManager.getRunningInstanceIds();
+ if (instanceIds.contains(instanceName)) {
+ return _pinotServiceManager.getInstanceStatus(instanceName);
+ }
+ throw new WebApplicationException(String.format("Instance [%s] not found.", instanceName),
+ Response.Status.NOT_FOUND);
+ }
+
+ @DELETE
Review comment:
added `stopAllPinotInstances` and `stopPinotInstancesByRole`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on issue #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on issue #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#issuecomment-617936122
We need to evaluate the backward incompatibility a bit on this one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#discussion_r429120606
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStatus.java
##########
@@ -43,17 +44,79 @@
*/
@SuppressWarnings("unused")
public class ServiceStatus {
+ public static final String STATUS_DESCRIPTION_NONE = "None";
+ public static final String STATUS_DESCRIPTION_INIT = "Init";
+ public static final String STATUS_DESCRIPTION_STARTED = "Started";
+ public static final String STATUS_DESCRIPTION_NO_HELIX_STATE = "Helix state does not exist";
private static final Logger LOGGER = LoggerFactory.getLogger(ServiceStatus.class);
+ private static final int MAX_RESOURCE_NAMES_TO_LOG = 5;
+ private static final Map<String, ServiceStatusCallback> serviceStatusCallbackMap = new ConcurrentHashMap<>();
+ private static final ServiceStatusCallback serviceStatusCallback =
+ new MapBasedMultipleCallbackServiceStatusCallback(serviceStatusCallbackMap);
- public enum Status {
- STARTING, GOOD, BAD
+ public static void setServiceStatusCallback(String name, ServiceStatusCallback serviceStatusCallback) {
+ ServiceStatus.serviceStatusCallbackMap.put(name, serviceStatusCallback);
}
- public static final String STATUS_DESCRIPTION_NONE = "None";
- public static final String STATUS_DESCRIPTION_INIT = "Init";
- public static final String STATUS_DESCRIPTION_NO_HELIX_STATE = "Helix state does not exist";
+ public static void removeServiceStatusCallback(String name) {
+ ServiceStatus.serviceStatusCallbackMap.remove(name);
+ }
- private static final int MAX_RESOURCE_NAMES_TO_LOG = 5;
+ public static Status getServiceStatus() {
+ return getServiceStatus(serviceStatusCallback);
+ }
+
+ public static Status getServiceStatus(String name) {
+ if (serviceStatusCallbackMap.containsKey(name)) {
+ return getServiceStatus(serviceStatusCallbackMap.get(name));
+ } else {
+ return Status.NOT_STARTED;
+ }
+ }
+
+ private static Status getServiceStatus(ServiceStatusCallback callback) {
+ try {
+ return callback.getServiceStatus();
+ } catch (Exception e) {
+ LOGGER.warn("Caught exception while reading the service status", e);
+ return Status.BAD;
+ }
+ }
+
+ public static String getStatusDescription() {
+ return getStatusDescription(serviceStatusCallback);
+ }
+
+ public static String getStatusDescription(String name) {
+ if (serviceStatusCallbackMap.containsKey(name)) {
+ return getStatusDescription(serviceStatusCallbackMap.get(name));
+ } else {
+ return STATUS_DESCRIPTION_NONE;
+ }
+ }
+
+ private static String getStatusDescription(ServiceStatusCallback callback) {
+ try {
+ return callback.getStatusDescription();
+ } catch (Exception e) {
+ return "Exception: " + e.getMessage();
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#issuecomment-626115475
@mcvsubbu Is this ok to go?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#discussion_r429111007
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
##########
@@ -0,0 +1,213 @@
+/**
+ * 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.pinot.tools.admin.command;
+
+import java.io.File;
+import java.net.SocketException;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.commons.configuration.Configuration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.tools.Command;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.apache.pinot.tools.utils.PinotConfigUtils;
+import org.kohsuke.args4j.Option;
+import org.kohsuke.args4j.spi.StringArrayOptionHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Helix.PINOT_SERVICE_ROLE;
+
+
+/**
+ * Class to implement StartPinotService command.
+ *
+ */
+public class StartServiceManagerCommand extends AbstractBaseAdminCommand implements Command {
+ private static final Logger LOGGER = LoggerFactory.getLogger(StartServiceManagerCommand.class);
+ private final List<Configuration> _bootstrapConfigurations = new ArrayList<>();
+ private final String[] BOOTSTRAP_SERVICES = new String[]{"CONTROLLER", "BROKER", "SERVER"};
+
+ @Option(name = "-help", required = false, help = true, aliases = {"-h", "--h", "--help"}, usage = "Print this message.")
+ private boolean _help;
+ @Option(name = "-zkAddress", required = true, metaVar = "<http>", usage = "Http address of Zookeeper.")
+ private String _zkAddress = DEFAULT_ZK_ADDRESS;
+ @Option(name = "-clusterName", required = true, metaVar = "<String>", usage = "Pinot cluster name.")
+ private String _clusterName = DEFAULT_CLUSTER_NAME;
+ @Option(name = "-port", required = true, metaVar = "<int>", usage = "Pinot service manager admin port, -1 means disable, 0 means a random available port.")
Review comment:
it's in `PinotServiceManager`
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/EnumArrayOptionHandler.java
##########
@@ -0,0 +1,67 @@
+/**
+ * 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.pinot.tools.admin.command;
+
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.OptionDef;
+import org.kohsuke.args4j.spi.OptionHandler;
+import org.kohsuke.args4j.spi.Parameters;
+import org.kohsuke.args4j.spi.Setter;
+
+
+public class EnumArrayOptionHandler<T extends Enum<T>> extends OptionHandler<T> {
Review comment:
this is not used. will delete
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/service/PinotServiceManagerAdminApiApplication.java
##########
@@ -0,0 +1,89 @@
+/**
+ * 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.pinot.tools.service;
+
+import com.google.common.base.Preconditions;
+import io.swagger.jaxrs.config.BeanConfig;
+import java.net.URI;
+import java.net.URL;
+import java.net.URLClassLoader;
+import org.glassfish.grizzly.http.server.CLStaticHttpHandler;
+import org.glassfish.grizzly.http.server.HttpHandler;
+import org.glassfish.grizzly.http.server.HttpServer;
+import org.glassfish.hk2.utilities.binding.AbstractBinder;
+import org.glassfish.jersey.grizzly2.httpserver.GrizzlyHttpServerFactory;
+import org.glassfish.jersey.jackson.JacksonFeature;
+import org.glassfish.jersey.server.ResourceConfig;
+
+
Review comment:
added
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
##########
@@ -0,0 +1,213 @@
+/**
+ * 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.pinot.tools.admin.command;
+
+import java.io.File;
+import java.net.SocketException;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.commons.configuration.Configuration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.tools.Command;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.apache.pinot.tools.utils.PinotConfigUtils;
+import org.kohsuke.args4j.Option;
+import org.kohsuke.args4j.spi.StringArrayOptionHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Helix.PINOT_SERVICE_ROLE;
+
+
+/**
+ * Class to implement StartPinotService command.
+ *
+ */
+public class StartServiceManagerCommand extends AbstractBaseAdminCommand implements Command {
+ private static final Logger LOGGER = LoggerFactory.getLogger(StartServiceManagerCommand.class);
+ private final List<Configuration> _bootstrapConfigurations = new ArrayList<>();
+ private final String[] BOOTSTRAP_SERVICES = new String[]{"CONTROLLER", "BROKER", "SERVER"};
+
+ @Option(name = "-help", required = false, help = true, aliases = {"-h", "--h", "--help"}, usage = "Print this message.")
+ private boolean _help;
+ @Option(name = "-zkAddress", required = true, metaVar = "<http>", usage = "Http address of Zookeeper.")
+ private String _zkAddress = DEFAULT_ZK_ADDRESS;
+ @Option(name = "-clusterName", required = true, metaVar = "<String>", usage = "Pinot cluster name.")
+ private String _clusterName = DEFAULT_CLUSTER_NAME;
+ @Option(name = "-port", required = true, metaVar = "<int>", usage = "Pinot service manager admin port, -1 means disable, 0 means a random available port.")
+ private int _port;
+ @Option(name = "-bootstrapConfigPaths", handler = StringArrayOptionHandler.class, required = false, usage = "A list of Pinot service config file paths. Each config file requires an extra config: 'pinot.service.role' to indicate which service to start.", forbids = {"-bootstrapServices"})
+ private String[] _bootstrapConfigPaths;
+ @Option(name = "-bootstrapServices", handler = StringArrayOptionHandler.class, required = false, usage = "A list of Pinot service roles to start with default config. E.g. CONTROLLER/BROKER/SERVER", forbids = {"-bootstrapConfigPaths"})
+ private String[] _bootstrapServices = BOOTSTRAP_SERVICES;
+
+ private PinotServiceManager _pinotServiceManager;
+
+ public String getZkAddress() {
+ return _zkAddress;
+ }
+
+ public StartServiceManagerCommand setZkAddress(String zkAddress) {
+ _zkAddress = zkAddress;
+ return this;
+ }
+
+ public String getClusterName() {
+ return _clusterName;
+ }
+
+ public StartServiceManagerCommand setClusterName(String clusterName) {
+ _clusterName = clusterName;
+ return this;
+ }
+
+ public int getPort() {
+ return _port;
+ }
+
+ public StartServiceManagerCommand setPort(int port) {
+ _port = port;
+ return this;
+ }
+
+ public String[] getBootstrapConfigPaths() {
+ return _bootstrapConfigPaths;
+ }
+
+ public StartServiceManagerCommand setBootstrapConfigPaths(String[] bootstrapConfigPaths) {
+ _bootstrapConfigPaths = bootstrapConfigPaths;
+ return this;
+ }
+
+ public String[] getBootstrapServices() {
+ return _bootstrapServices;
+ }
+
+ public StartServiceManagerCommand setBootstrapServices(String[] bootstrapServices) {
+ _bootstrapServices = bootstrapServices;
+ return this;
+ }
+
+ @Override
+ public boolean getHelp() {
+ return _help;
+ }
+
+ public void setHelp(boolean help) {
+ _help = help;
+ }
+
+ @Override
+ public String getName() {
+ return "StartPinotService";
+ }
+
+ @Override
+ public String toString() {
+ StringBuilder sb = new StringBuilder()
+ .append("StartServiceManager -clusterName " + _clusterName + " -zkAddress " + _zkAddress + " -port " + _port);
+ if (_bootstrapConfigPaths != null) {
+ sb.append(" -bootstrapConfigPaths " + Arrays.toString(_bootstrapConfigPaths));
+ } else if (_bootstrapServices != null) {
+ sb.append(" -bootstrapServices " + Arrays.toString(_bootstrapServices));
+ }
+
+ return sb.toString();
+ }
+
+ @Override
+ public void cleanup() {
+ }
+
+ @Override
+ public String description() {
+ return "Start the Pinot Service Process at the specified port.";
Review comment:
added
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on issue #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#issuecomment-617499901
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=h1) Report
> Merging [#5266](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/bd79861e7bca87406fc184ec70ce504667f76daf&el=desc) will **decrease** coverage by `0.18%`.
> The diff coverage is `68.75%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5266 +/- ##
==========================================
- Coverage 66.23% 66.05% -0.19%
==========================================
Files 1066 1068 +2
Lines 54552 54599 +47
Branches 8128 8133 +5
==========================================
- Hits 36132 36064 -68
- Misses 15774 15893 +119
+ Partials 2646 2642 -4
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `76.66% <ø> (ø)` | |
| [.../BrokerResourceOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclJlc291cmNlT25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `55.81% <ø> (ø)` | |
| [...quota/HelixExternalViewBasedQueryQuotaManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IZWxpeEV4dGVybmFsVmlld0Jhc2VkUXVlcnlRdW90YU1hbmFnZXIuamF2YQ==) | `67.87% <ø> (ø)` | |
| [...roker/requesthandler/BaseBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvQmFzZUJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `79.35% <ø> (ø)` | |
| [...rg/apache/pinot/broker/routing/RoutingManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9Sb3V0aW5nTWFuYWdlci5qYXZh) | `81.15% <ø> (ø)` | |
| [...ting/instanceselector/InstanceSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3JGYWN0b3J5LmphdmE=) | `71.42% <ø> (ø)` | |
| [...er/routing/segmentpruner/SegmentPrunerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1NlZ21lbnRQcnVuZXJGYWN0b3J5LmphdmE=) | `83.33% <ø> (ø)` | |
| [...outing/segmentselector/SegmentSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50c2VsZWN0b3IvU2VnbWVudFNlbGVjdG9yRmFjdG9yeS5qYXZh) | `60.00% <ø> (ø)` | |
| [...oker/routing/timeboundary/TimeBoundaryManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy90aW1lYm91bmRhcnkvVGltZUJvdW5kYXJ5TWFuYWdlci5qYXZh) | `87.50% <ø> (ø)` | |
| [...mmon/assignment/InstanceAssignmentConfigUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZUFzc2lnbm1lbnRDb25maWdVdGlscy5qYXZh) | `67.50% <ø> (-5.00%)` | :arrow_down: |
| ... and [148 more](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=footer). Last update [79cc2a9...31c6bf4](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#issuecomment-617499901
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=h1) Report
> Merging [#5266](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/bd79861e7bca87406fc184ec70ce504667f76daf&el=desc) will **decrease** coverage by `0.18%`.
> The diff coverage is `68.75%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5266 +/- ##
==========================================
- Coverage 66.23% 66.05% -0.19%
==========================================
Files 1066 1068 +2
Lines 54552 54599 +47
Branches 8128 8133 +5
==========================================
- Hits 36132 36064 -68
- Misses 15774 15893 +119
+ Partials 2646 2642 -4
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `76.66% <ø> (ø)` | |
| [.../BrokerResourceOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclJlc291cmNlT25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `55.81% <ø> (ø)` | |
| [...quota/HelixExternalViewBasedQueryQuotaManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IZWxpeEV4dGVybmFsVmlld0Jhc2VkUXVlcnlRdW90YU1hbmFnZXIuamF2YQ==) | `67.87% <ø> (ø)` | |
| [...roker/requesthandler/BaseBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvQmFzZUJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `79.35% <ø> (ø)` | |
| [...rg/apache/pinot/broker/routing/RoutingManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9Sb3V0aW5nTWFuYWdlci5qYXZh) | `81.15% <ø> (ø)` | |
| [...ting/instanceselector/InstanceSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3JGYWN0b3J5LmphdmE=) | `71.42% <ø> (ø)` | |
| [...er/routing/segmentpruner/SegmentPrunerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1NlZ21lbnRQcnVuZXJGYWN0b3J5LmphdmE=) | `83.33% <ø> (ø)` | |
| [...outing/segmentselector/SegmentSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50c2VsZWN0b3IvU2VnbWVudFNlbGVjdG9yRmFjdG9yeS5qYXZh) | `60.00% <ø> (ø)` | |
| [...oker/routing/timeboundary/TimeBoundaryManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy90aW1lYm91bmRhcnkvVGltZUJvdW5kYXJ5TWFuYWdlci5qYXZh) | `87.50% <ø> (ø)` | |
| [...mmon/assignment/InstanceAssignmentConfigUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZUFzc2lnbm1lbnRDb25maWdVdGlscy5qYXZh) | `67.50% <ø> (-5.00%)` | :arrow_down: |
| ... and [148 more](https://codecov.io/gh/apache/incubator-pinot/pull/5266/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=footer). Last update [79cc2a9...31c6bf4](https://codecov.io/gh/apache/incubator-pinot/pull/5266?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#discussion_r429111535
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java
##########
@@ -0,0 +1,213 @@
+/**
+ * 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.pinot.tools.admin.command;
+
+import java.io.File;
+import java.net.SocketException;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.commons.configuration.Configuration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.tools.Command;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.apache.pinot.tools.utils.PinotConfigUtils;
+import org.kohsuke.args4j.Option;
+import org.kohsuke.args4j.spi.StringArrayOptionHandler;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Helix.PINOT_SERVICE_ROLE;
+
+
+/**
+ * Class to implement StartPinotService command.
+ *
+ */
+public class StartServiceManagerCommand extends AbstractBaseAdminCommand implements Command {
Review comment:
added examples
##########
File path: pinot-tools/src/main/java/org/apache/pinot/tools/service/api/resources/PinotServiceManagerInstanceResource.java
##########
@@ -0,0 +1,241 @@
+/**
+ * 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.pinot.tools.service.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.NetUtil;
+import org.apache.pinot.controller.ControllerConf;
+import org.apache.pinot.spi.services.ServiceRole;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.apache.pinot.tools.service.PinotServiceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.common.utils.CommonConstants.Controller.CONFIG_OF_CONTROLLER_METRICS_PREFIX;
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#issuecomment-631702902
can we list the commands needed to start Pinot after this gets in?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#discussion_r429422165
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/services/ServiceStartable.java
##########
@@ -0,0 +1,40 @@
+/**
+ * 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.pinot.spi.services;
+
+import org.apache.commons.configuration.Configuration;
+
+
+/**
+ * ServiceStartable is the general interface to manage a Pinot instance lifecycle for a specific ServiceRole.
+ * E.g. Controller/Broker/Server/Minion.
+ *
+ */
+public interface ServiceStartable {
Review comment:
Changed to StartableService
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on a change in pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#discussion_r430928817
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStatus.java
##########
@@ -43,17 +44,79 @@
*/
@SuppressWarnings("unused")
public class ServiceStatus {
+ public static final String STATUS_DESCRIPTION_NONE = "None";
+ public static final String STATUS_DESCRIPTION_INIT = "Init";
+ public static final String STATUS_DESCRIPTION_STARTED = "Started";
+ public static final String STATUS_DESCRIPTION_NO_HELIX_STATE = "Helix state does not exist";
private static final Logger LOGGER = LoggerFactory.getLogger(ServiceStatus.class);
+ private static final int MAX_RESOURCE_NAMES_TO_LOG = 5;
+ private static final Map<String, ServiceStatusCallback> serviceStatusCallbackMap = new ConcurrentHashMap<>();
+ private static final ServiceStatusCallback serviceStatusCallback =
+ new MapBasedMultipleCallbackServiceStatusCallback(serviceStatusCallbackMap);
- public enum Status {
- STARTING, GOOD, BAD
+ public static void setServiceStatusCallback(String name, ServiceStatusCallback serviceStatusCallback) {
+ ServiceStatus.serviceStatusCallbackMap.put(name, serviceStatusCallback);
}
- public static final String STATUS_DESCRIPTION_NONE = "None";
- public static final String STATUS_DESCRIPTION_INIT = "Init";
- public static final String STATUS_DESCRIPTION_NO_HELIX_STATE = "Helix state does not exist";
+ public static void removeServiceStatusCallback(String name) {
+ ServiceStatus.serviceStatusCallbackMap.remove(name);
+ }
- private static final int MAX_RESOURCE_NAMES_TO_LOG = 5;
+ public static Status getServiceStatus() {
+ return getServiceStatus(serviceStatusCallback);
+ }
+
+ public static Status getServiceStatus(String name) {
+ if (serviceStatusCallbackMap.containsKey(name)) {
+ return getServiceStatus(serviceStatusCallbackMap.get(name));
+ } else {
+ return Status.NOT_STARTED;
Review comment:
Got it. Added `SHUTTING_DOWN` status.
My feeling is that during shutting down phase, the `getServiceStatus(serviceStatusCallbackMap.get(name))` should return `SHUTTING_DOWN`.
Once server is completely shutdown, it will be removed from the `serviceStatusCallbackMap` so the status will be `NOT_STARTED`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#discussion_r430838191
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStatus.java
##########
@@ -43,17 +44,79 @@
*/
@SuppressWarnings("unused")
public class ServiceStatus {
+ public static final String STATUS_DESCRIPTION_NONE = "None";
+ public static final String STATUS_DESCRIPTION_INIT = "Init";
+ public static final String STATUS_DESCRIPTION_STARTED = "Started";
+ public static final String STATUS_DESCRIPTION_NO_HELIX_STATE = "Helix state does not exist";
private static final Logger LOGGER = LoggerFactory.getLogger(ServiceStatus.class);
+ private static final int MAX_RESOURCE_NAMES_TO_LOG = 5;
+ private static final Map<String, ServiceStatusCallback> serviceStatusCallbackMap = new ConcurrentHashMap<>();
+ private static final ServiceStatusCallback serviceStatusCallback =
+ new MapBasedMultipleCallbackServiceStatusCallback(serviceStatusCallbackMap);
- public enum Status {
- STARTING, GOOD, BAD
+ public static void setServiceStatusCallback(String name, ServiceStatusCallback serviceStatusCallback) {
+ ServiceStatus.serviceStatusCallbackMap.put(name, serviceStatusCallback);
}
- public static final String STATUS_DESCRIPTION_NONE = "None";
- public static final String STATUS_DESCRIPTION_INIT = "Init";
- public static final String STATUS_DESCRIPTION_NO_HELIX_STATE = "Helix state does not exist";
+ public static void removeServiceStatusCallback(String name) {
+ ServiceStatus.serviceStatusCallbackMap.remove(name);
+ }
- private static final int MAX_RESOURCE_NAMES_TO_LOG = 5;
+ public static Status getServiceStatus() {
+ return getServiceStatus(serviceStatusCallback);
+ }
+
+ public static Status getServiceStatus(String name) {
+ if (serviceStatusCallbackMap.containsKey(name)) {
+ return getServiceStatus(serviceStatusCallbackMap.get(name));
+ } else {
+ return Status.NOT_STARTED;
Review comment:
But then the shut down sequence is not instantaneous. You call shutdown and then wait for it to shutdown. During that time, the status should be SHUTTING_DOWN. And then you can change it to NOT_STARTED when the shutdown is complete.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 merged pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 merged pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on pull request #5266: Add PinotServiceManager to start Pinot components
Posted by GitBox <gi...@apache.org>.
fx19880617 commented on pull request #5266:
URL: https://github.com/apache/incubator-pinot/pull/5266#issuecomment-634823281
> We will fix the incompat at our end. If for some reason we need some changes, we will come back with those. thanks
Thanks for taking time to review on this !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org