Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FISH-898 Added timeout option to instance commands #5630

Merged
merged 5 commits into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public class RemoteCLICommand extends CLICommand {
private RemoteCLICommand.CLIRemoteAdminCommand rac;
private final MultiMap<String, AdminCommandListener<GfSseInboundEvent>> listeners =
new MultiMap<String, AdminCommandListener<GfSseInboundEvent>>();
private int readTimeout;

/**
* A special RemoteAdminCommand that overrides methods so that we can handle
Expand Down Expand Up @@ -156,6 +157,15 @@ public CLIRemoteAdminCommand(String name, String host, int port, boolean secure,
super.setReadTimeout(Integer.parseInt(stimeout));
}
}

/**
* Set the RemoteRestCommand read timeout
* @param readTimeout timeout in milliseconds
*/
@Override
public void setReadTimeout(int readTimeout){
super.setReadTimeout(readTimeout);
}

@Override
public void fetchCommandModel() throws CommandException {
Expand Down Expand Up @@ -924,6 +934,9 @@ private void initializeRemoteAdminCommand() throws CommandException {
programOpts.getPassword(), logger, programOpts.getAuthToken(),programOpts.isNotifyCommand());
rac.setFileOutputDirectory(outputDir);
rac.setInteractive(programOpts.isInteractive());
if(readTimeout > 0){
kalinchan marked this conversation as resolved.
Show resolved Hide resolved
rac.setReadTimeout(readTimeout);
}
for (String key : listeners.keySet()) {
for (AdminCommandListener<GfSseInboundEvent> listener : listeners.get(key)) {
rac.registerListener(key, listener);
Expand Down Expand Up @@ -1026,4 +1039,12 @@ private static synchronized ClassLoader getModuleClassLoader() {
moduleClassLoader = new DirectoryClassLoader(modulesDir, CLICommand.class.getClassLoader());
return moduleClassLoader;
}

/**
* Set read timeout for RemoteRestCommand
* @param readTimeout
*/
public void setReadTimeout(int readTimeout) {
this.readTimeout = readTimeout;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* holder.
*/

// Portions Copyright [2016-2021] [Payara Foundation and/or affiliates]
// Portions Copyright [2016-2022] [Payara Foundation and/or affiliates]

package com.sun.enterprise.admin.servermgmt.cli;

Expand Down Expand Up @@ -409,23 +409,27 @@ protected boolean isRunning() {
}
}

/**
* Byron Nevins Says: We have quite a historical assortment of ways to determine
* if a server has restarted. There are little teeny timing issues with all of
* them. I'm confident that this new technique will clear them all up. Here we
* are just monitoring the PID of the new server and comparing it to the pid of
* the old server. The oldServerPid is guaranteed to be either the PID of the
* "old" server or -1 if we couldn't get it -- or it isn't running. If it is -1
* then we make the assumption that once we DO get a valid pid that the server
* has started. If the old pid is valid we simply poll until we get a different
* pid. Notice that we will never get a valid pid back unless the server is
* officially up and running and "STARTED" Created April 2013
*
* @param oldServerPid The pid of the server which is being restarted.
* @throws CommandException if we time out.
*/
protected final void waitForRestart(final int oldServerPid) throws CommandException {
long end = getEndTime();
waitForRestart(oldServerPid, CLIConstants.WAIT_FOR_DAS_TIME_MS);
}

/**
* Byron Nevins Says: We have quite a historical assortment of ways to determine
* if a server has restarted. There are little teeny timing issues with all of
* them. I'm confident that this new technique will clear them all up. Here we
* are just monitoring the PID of the new server and comparing it to the pid of
* the old server. The oldServerPid is guaranteed to be either the PID of the
* "old" server or -1 if we couldn't get it -- or it isn't running. If it is -1
* then we make the assumption that once we DO get a valid pid that the server
* has started. If the old pid is valid we simply poll until we get a different
* pid. Notice that we will never get a valid pid back unless the server is
* officially up and running and "STARTED" Created April 2013
*
* @param oldServerPid The pid of the server which is being restarted.
* @throws CommandException if we time out.
*/
protected final void waitForRestart(final int oldServerPid, long timeout) throws CommandException {
long end = getEndTime(timeout);

while (now() < end) {
try {
Expand Down Expand Up @@ -608,10 +612,10 @@ private long now() {
return System.currentTimeMillis();
}

private long getEndTime() {
private long getEndTime(long timeout) {
// it's a method in case we someday allow configuring this VERY long
// timeout at runtime.
kalinchan marked this conversation as resolved.
Show resolved Hide resolved
return CLIConstants.WAIT_FOR_DAS_TIME_MS + now();
return timeout + now();
}

protected boolean dataGridEncryptionEnabled() throws IOException, XMLStreamException {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
*
* Copyright (c) 2022 Payara Foundation and/or its affiliates. All rights reserved.
*
* The contents of this file are subject to the terms of either the GNU
* General Public License Version 2 only ("GPL") or the Common Development
* and Distribution License("CDDL") (collectively, the "License"). You
* may not use this file except in compliance with the License. You can
* obtain a copy of the License at
* https://github.com/payara/Payara/blob/master/LICENSE.txt
* See the License for the specific
* language governing permissions and limitations under the License.
*
* When distributing the software, include this License Header Notice in each
* file and include the License file at glassfish/legal/LICENSE.txt.
*
* GPL Classpath Exception:
* The Payara Foundation designates this particular file as subject to the "Classpath"
* exception as provided by the Payara Foundation in the GPL Version 2 section of the License
* file that accompanied this code.
*
* Modifications:
* If applicable, add the following below the License Header, with the fields
* enclosed by brackets [] replaced by your own identifying information:
* "Portions Copyright [year] [name of copyright owner]"
*
* Contributor(s):
* If you wish your version of this file to be governed by only the CDDL or
* only the GPL Version 2, indicate your decision by adding "[Contributor]
* elects to include this software in this distribution under the [CDDL or GPL
* Version 2] license." If you don't indicate a single choice of license, a
* recipient has the option to distribute your version of this file under
* either the CDDL, the GPL Version 2 or to extend the choice of license to
* its licensees as provided above. However, if you add GPL Version 2 code
* and therefore, elected the GPL Version 2 license, then the option applies
* only if the new code is made subject to such option by the copyright
* holder.
*/

package com.sun.enterprise.admin.util;

import com.sun.enterprise.admin.remote.RemoteRestAdminCommand;
import org.glassfish.api.ExecutionContext;
import org.glassfish.api.ParamDefaultCalculator;

/**
* Class to use within the @Param annotation for a calculator to get the timeout
* from either a System Property, Environment Property or a constant value
*
* @author Kalin Chan
*/

kalinchan marked this conversation as resolved.
Show resolved Hide resolved
public class TimeoutParamDefaultCalculator extends ParamDefaultCalculator {

public TimeoutParamDefaultCalculator() {
}

@Override
public String defaultValue(ExecutionContext context) {
return String.valueOf(RemoteRestAdminCommand.getReadTimeout() / 1000) ;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* holder.
*/

// Portions Copyright [2018-2019] [Payara Foundation and/or its affiliates]
// Portions Copyright [2018-2022] [Payara Foundation and/or its affiliates]

package com.sun.enterprise.v3.admin.cluster;

Expand Down Expand Up @@ -92,6 +92,7 @@ public class ClusterCommandHelper {
private final CommandRunner runner;

private ProgressStatus progress;
private long adminTimeout;

/**
* Construct a ClusterCommandHelper
Expand All @@ -102,6 +103,7 @@ public class ClusterCommandHelper {
public ClusterCommandHelper(Domain domain, CommandRunner runner) {
this.domain = domain;
this.runner = runner;
this.adminTimeout = RemoteRestAdminCommand.getReadTimeout() - 3000;
kalinchan marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -238,14 +240,6 @@ public ActionReport runCommand(String command, ParameterMap map, String targetNa
if (logger.isLoggable(FINE)) {
logger.fine(String.format("%s commands queued, waiting for responses", command));
}

// Make sure we don't wait longer than the admin read timeout. Set
// our limit to be 3 seconds less.
long adminTimeout = RemoteRestAdminCommand.getReadTimeout() - 3000;
if (adminTimeout <= 0) {
// This should never be the case
adminTimeout = 57 * 1000;
}

if (logger.isLoggable(FINE)) {
logger.fine(String.format("Initial cluster command timeout: %d ms", adminTimeout));
Expand Down Expand Up @@ -436,4 +430,12 @@ public static class ReportResult {
public final List<String> succeededServerNames = new ArrayList<>();
public final List<String> failedServerNames = new ArrayList<>();
}

/**
* Set the timeout for ClusterCommandHelper
* @param adminTimeout in milliseconds
*/
public void setAdminTimeout(long adminTimeout) {
kalinchan marked this conversation as resolved.
Show resolved Hide resolved
this.adminTimeout = adminTimeout;
kalinchan marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
# only if the new code is made subject to such option by the copyright
# holder.
#
## Portions Copyright [2018-2019] [Payara Foundation and/or its affiliates]
## Portions Copyright [2018-2022] [Payara Foundation and/or its affiliates]

#####restart-instance
restart.instance.notInstance=-_restart-instance only works on instances. This is a {0}
Expand Down Expand Up @@ -152,6 +152,7 @@ start.dg.command=Start a deployment group
start.dg=Starting deployment group {0}
stop.dg=Stopping deployment group {0}
restart.dg=Restarting deployment group {0}
restart.dg.timeout=Timed out while waiting for deployment group {0} to restart
## StartClusterCommand
start.cluster.command=Start a cluster
start.cluster=Starting cluster {0}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,26 +37,24 @@
* only if the new code is made subject to such option by the copyright
* holder.
*
* Portions Copyright [2016-2018] [Payara Foundation and/or its affiliates]
* Portions Copyright [2016-2022] [Payara Foundation and/or its affiliates]
*
*/

package com.sun.enterprise.v3.admin.cluster;

import com.sun.enterprise.admin.util.TimeoutParamDefaultCalculator;
import com.sun.enterprise.config.serverbeans.Cluster;
import java.util.logging.Logger;

import org.glassfish.api.admin.*;
import javax.inject.Inject;


import org.jvnet.hk2.annotations.Service;
import org.glassfish.api.Param;
import com.sun.enterprise.config.serverbeans.Domain;
import org.glassfish.api.ActionReport;
import org.glassfish.api.ActionReport.ExitCode;
import org.glassfish.api.Param;
import org.glassfish.api.admin.*;
import org.glassfish.hk2.api.PerLookup;
import org.jvnet.hk2.annotations.Service;

import com.sun.enterprise.config.serverbeans.Domain;
import javax.inject.Inject;
import java.util.logging.Logger;

@Service(name = "restart-cluster")
@ExecuteOn(value={RuntimeType.DAS})
Expand Down Expand Up @@ -88,19 +86,33 @@ public class RestartClusterCommand implements AdminCommand {

@Param(optional = true, defaultValue = "false")
private boolean verbose;

@Param(optional = true, defaultValue = "true")
private boolean rolling;

@Param(optional = true, defaultValue = "0")
private String delay;

@Param(optional = true, defaultCalculator = TimeoutParamDefaultCalculator.class)
private int instanceTimeout;

@Param(optional = true, defaultCalculator = TimeoutParamDefaultCalculator.class)
private int timeout;

@Override
public void execute(AdminCommandContext context) {

ActionReport report = context.getActionReport();
Logger logger = context.getLogger();

if (timeout <= 0 || instanceTimeout <= 0) {
kalinchan marked this conversation as resolved.
Show resolved Hide resolved
String msg = "Timeout must be at least 1 second long.";
logger.warning(msg);
report.setActionExitCode(ActionReport.ExitCode.FAILURE);
report.setMessage(msg);
return;
}

logger.info(Strings.get("restart.cluster", clusterName));

// Require that we be a DAS
Expand All @@ -120,6 +132,8 @@ public void execute(AdminCommandContext context) {
String commandName = "restart-instance";
ParameterMap pm = new ParameterMap();
pm.add("delay", delay);
pm.add("timeout", String.valueOf(instanceTimeout));
clusterHelper.setAdminTimeout(timeout * 1000);
kalinchan marked this conversation as resolved.
Show resolved Hide resolved
clusterHelper.runCommand(commandName, pm, clusterName, context,
verbose, rolling);
}
Expand Down
Loading