modules/core/domain/src/main/java/org/rhq/core/domain/plugin/ServerPlugin.java | 10 +++ modules/core/domain/src/test/java/org/rhq/core/domain/resource/test/ServerPluginTest.java | 15 +++++ modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/admin/plugin/EditPluginConfigurationUIBean.java | 15 ++--- modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/plugin/ServerPluginScanner.java | 27 ++++++++-- modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsBean.java | 17 ++++++ modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsLocal.java | 11 ++++ modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/AbstractTypeServerPluginContainer.java | 23 ++++++++ modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/ServerPluginsBeanTest.java | 24 ++++++++ 8 files changed, 128 insertions(+), 14 deletions(-)
New commits: commit 987ee2cf875b01c80d4ff671d1a5abf154042529 Author: John Mazzitelli mazz@redhat.com Date: Fri Jan 29 02:54:50 2010 -0500
fixes BZ#548526 and obsoletes BZ#559160 - when changing config of server plugin, plugin is hot-redeployed and other HA servers will pick up the chanage within a few minutes at most
diff --git a/modules/core/domain/src/main/java/org/rhq/core/domain/plugin/ServerPlugin.java b/modules/core/domain/src/main/java/org/rhq/core/domain/plugin/ServerPlugin.java index 4a8f9f5..6aaf7cf 100644 --- a/modules/core/domain/src/main/java/org/rhq/core/domain/plugin/ServerPlugin.java +++ b/modules/core/domain/src/main/java/org/rhq/core/domain/plugin/ServerPlugin.java @@ -229,6 +229,15 @@ import org.rhq.core.domain.configuration.Configuration; + " WHERE p.id IN (:ids) " // + " AND p.status = 'INSTALLED' "), //
+ // returns two epoch millis - when plugin config and schedule jobs were last changed + @NamedQuery(name = ServerPlugin.QUERY_GET_CONFIG_MTIMES, query = "" // + + " SELECT pc.mtime, " // + + " sjc.mtime " // + + " FROM ServerPlugin AS p " // + + " LEFT JOIN p.pluginConfiguration pc " // + + " LEFT JOIN p.scheduledJobsConfiguration sjc " // + + " WHERE p.id = :id"), // + // this query is how you enable and disable plugins @NamedQuery(name = ServerPlugin.UPDATE_PLUGIN_ENABLED_BY_ID, query = "" // + "UPDATE ServerPlugin p " // @@ -250,6 +259,7 @@ public class ServerPlugin extends AbstractPlugin { public static final String QUERY_FIND_ALL_INSTALLED = "ServerPlugin.findAllInstalled"; public static final String QUERY_FIND_ALL_INSTALLED_KEYS = "ServerPlugin.findAllInstalledKeys"; public static final String QUERY_FIND_KEYS_BY_IDS = "ServerPlugin.findKeysByIds"; + public static final String QUERY_GET_CONFIG_MTIMES = "ServerPlugin.getConfigMTimes"; public static final String UPDATE_PLUGIN_ENABLED_BY_ID = "ServerPlugin.updatePluginEnabledById";
@JoinColumn(name = "JOBS_CONFIG_ID", referencedColumnName = "ID") diff --git a/modules/core/domain/src/test/java/org/rhq/core/domain/resource/test/ServerPluginTest.java b/modules/core/domain/src/test/java/org/rhq/core/domain/resource/test/ServerPluginTest.java index 1e52b62..4ec9271 100644 --- a/modules/core/domain/src/test/java/org/rhq/core/domain/resource/test/ServerPluginTest.java +++ b/modules/core/domain/src/test/java/org/rhq/core/domain/resource/test/ServerPluginTest.java @@ -85,6 +85,12 @@ public class ServerPluginTest extends AbstractEJB3Test { assert plugin.getPluginConfiguration() == null : "there was no config that should have been here"; assert plugin.getScheduledJobsConfiguration() == null : "there was no config that should have been here";
+ q = em.createNamedQuery(ServerPlugin.QUERY_GET_CONFIG_MTIMES); + q.setParameter("id", plugin.getId()); + Object[] times = (Object[]) q.getSingleResult(); + assert times[0] == null; + assert times[1] == null; + q = em.createNamedQuery(ServerPlugin.QUERY_GET_STATUS_BY_NAME); q.setParameter("name", plugin.getName()); assert ((PluginStatusType) q.getSingleResult()) == PluginStatusType.INSTALLED; @@ -270,6 +276,7 @@ public class ServerPluginTest extends AbstractEJB3Test { } }
+ @SuppressWarnings("unchecked") public void testPersistFull() throws Exception { getTransactionManager().begin(); EntityManager em = getEntityManager(); @@ -410,6 +417,14 @@ public class ServerPluginTest extends AbstractEJB3Test { assert allKeys.size() == 1; assert allKeys.contains(new PluginKey(plugin));
+ query = em.createNamedQuery(ServerPlugin.QUERY_GET_CONFIG_MTIMES); + query.setParameter("id", plugin.getId()); + Object[] times = (Object[]) query.getSingleResult(); + assert times[0] != null; + assert times[1] != null; + assert ((Long) times[0]).longValue() > 0; + assert ((Long) times[1]).longValue() > 0; + // mark a plugin deleted - all of our queries should then never see it plugin.setStatus(PluginStatusType.DELETED); em.merge(plugin); diff --git a/modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/admin/plugin/EditPluginConfigurationUIBean.java b/modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/admin/plugin/EditPluginConfigurationUIBean.java index 34b24a6..ad6c709 100644 --- a/modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/admin/plugin/EditPluginConfigurationUIBean.java +++ b/modules/enterprise/gui/portal-war/src/main/java/org/rhq/enterprise/gui/admin/plugin/EditPluginConfigurationUIBean.java @@ -112,16 +112,13 @@ public class EditPluginConfigurationUIBean extends AbstractPluginConfigurationUI ServerPlugin plugin = serverPlugins.updateServerPluginExceptContent(subject, getPlugin()); setPlugin(plugin);
- // the config has changed. we would like to restart the plugin, but we would need to ensure - // ALL servers in the rhq server cloud restart. today we have no easy way to inform all servers - // that they should do this. So, for now, disable the plugin (since if it is running, it is doing - // so with a now-invalid configuration. Let the user go in and re-enable the plugin later. - // TODO: right now, if the user re-enables the plugin too fast, some of the other servers - // will not have seen the state change and won't restart the plugin. Need a way to do this... - // perhaps use the mtime column to denote that state changed (this would affect whether or not - // the plugin needs to be upgraded, so we need to be careful re-using the mtime column for this purpose). + // Since the config has changed, we can tell the server to re-load the plugin now + // in order for it to pick up the changes immediately. Any other servers in the HA Server Cloud + // will pick up these changes later, when they scan for changes in the database. + // Note that if the plugin is disabled, don't bother since the plugin isn't really running anyway. if (plugin.isEnabled()) { - serverPlugins.disableServerPlugins(subject, getPluginIdList()); + // enabling an already enabled plugin forces the plugin to reload + serverPlugins.enableServerPlugins(subject, getPluginIdList()); }
FacesContextUtility.addMessage(FacesMessage.SEVERITY_INFO, "Configuration settings saved."); diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/plugin/ServerPluginScanner.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/plugin/ServerPluginScanner.java index a58afb2..6b15cdc 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/plugin/ServerPluginScanner.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/core/plugin/ServerPluginScanner.java @@ -106,11 +106,11 @@ public class ServerPluginScanner { List<ServerPlugin> allPlugins = serverPluginsManager.getAllServerPlugins(); List<ServerPlugin> installedPlugins = new ArrayList<ServerPlugin>(); List<ServerPlugin> undeployedPlugins = new ArrayList<ServerPlugin>(); - for (ServerPlugin allPlugin : allPlugins) { - if (allPlugin.getStatus() == PluginStatusType.INSTALLED) { - installedPlugins.add(allPlugin); + for (ServerPlugin plugin : allPlugins) { + if (plugin.getStatus() == PluginStatusType.INSTALLED) { + installedPlugins.add(plugin); } else { - undeployedPlugins.add(allPlugin); + undeployedPlugins.add(plugin); } }
@@ -130,16 +130,33 @@ public class ServerPluginScanner { }
// now see if any plugins changed state from enabled->disabled or vice versa + // this also checks to see if the plugin configuration has changed since it was last loaded MasterServerPluginContainer master = LookupUtil.getServerPluginService().getMasterPluginContainer(); if (master != null) { for (ServerPlugin installedPlugin : installedPlugins) { PluginKey key = PluginKey.createServerPluginKey(installedPlugin.getType(), installedPlugin.getName()); AbstractTypeServerPluginContainer pc = master.getPluginContainerByPlugin(key); if (pc != null && pc.isPluginLoaded(key)) { + boolean needToReloadPlugin = false; boolean currentlyEnabled = pc.isPluginEnabled(key); if (installedPlugin.isEnabled() != currentlyEnabled) { - log.info("Detected a change to plugin [" + key + "]. It will now be " + log.info("Detected a state change to plugin [" + key + "]. It will now be " + ((installedPlugin.isEnabled()) ? "[enabled]" : "[disabled]")); + needToReloadPlugin = true; + } else { + Long pluginLoadTime = pc.getPluginLoadTime(key); + if (pluginLoadTime != null) { + long configChangeTimestamp = serverPluginsManager + .getLastConfigurationChangeTimestamp(installedPlugin.getId()); + if (configChangeTimestamp > pluginLoadTime.longValue()) { + // since the last time the plugin was loaded, its configuration has changed, reload it to pick up the new config + log.info("Detected a config change to plugin [" + key + "]. It will be reloaded and " + + ((installedPlugin.isEnabled()) ? "[enabled]" : "[disabled]")); + needToReloadPlugin = true; + } + } + } + if (needToReloadPlugin) { pc.reloadPlugin(key, installedPlugin.isEnabled()); } } diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsBean.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsBean.java index 4d7d590..a4103c4 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsBean.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsBean.java @@ -147,6 +147,23 @@ public class ServerPluginsBean implements ServerPluginsLocal { return query.getResultList(); }
+ @SuppressWarnings("unchecked") + public long getLastConfigurationChangeTimestamp(int pluginId) { + Query query = entityManager.createNamedQuery(ServerPlugin.QUERY_GET_CONFIG_MTIMES); + query.setParameter("id", pluginId); + Object[] timestamps = (Object[]) query.getSingleResult(); + long lastConfigChangeTimestamp = 0L; + for (Object timestamp : timestamps) { + if (timestamp != null) { + long val = ((Long) timestamp).longValue(); + if (val > lastConfigChangeTimestamp) { + lastConfigChangeTimestamp = val; + } + } + } + return lastConfigChangeTimestamp; + } + public ServerPluginDescriptorType getServerPluginDescriptor(PluginKey pluginKey) throws Exception { ServerPlugin plugin = getServerPlugin(pluginKey); File pluginsDir = LookupUtil.getServerPluginService().getServerPluginsDirectory(); diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsLocal.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsLocal.java index 6a81f29..830b2c5 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsLocal.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/ServerPluginsLocal.java @@ -90,6 +90,17 @@ public interface ServerPluginsLocal { List<ServerPlugin> getAllServerPluginsById(List<Integer> pluginIds);
/** + * Given a plugin ID, this will return a timestamp (in epoch millis) + * that indicates the last time when the plugin's configuration changed. + * This looks at both plugin configuration and schedule job configuration. + * + * @param pluginId + * @return time when the plugin's configuration was last updated; will be 0 + * if the plugin has no configuration to change. + */ + long getLastConfigurationChangeTimestamp(int pluginId); + + /** * Given a plugin key, returns the descriptor for that plugin. * * @param pluginKey diff --git a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/AbstractTypeServerPluginContainer.java b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/AbstractTypeServerPluginContainer.java index f845715..b8e760c 100644 --- a/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/AbstractTypeServerPluginContainer.java +++ b/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/plugin/pc/AbstractTypeServerPluginContainer.java @@ -20,7 +20,10 @@ package org.rhq.enterprise.server.plugin.pc;
import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map;
import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -48,6 +51,11 @@ public abstract class AbstractTypeServerPluginContainer { private ServerPluginManager pluginManager;
/** + * Maps the time (in epoch milliseconds) when plugins were loaded in this plugin container. + */ + private final Map<PluginKey, Long> loadedTimestamps; + + /** * Instantiates the plugin container. All subclasses must support this and only this * constructor. * @@ -55,6 +63,7 @@ public abstract class AbstractTypeServerPluginContainer { */ public AbstractTypeServerPluginContainer(MasterServerPluginContainer master) { this.master = master; + this.loadedTimestamps = Collections.synchronizedMap(new HashMap<PluginKey, Long>()); }
/** @@ -111,6 +120,17 @@ public abstract class AbstractTypeServerPluginContainer { }
/** + * Given a plugin key, this returns the time (in epoch milliseconds) when that plugin was loaded into + * this plugin container. + * + * @param pluginKey identifies the plugin whose load time is to be returned + * @return the epoch millis timestamp when the plugin was loaded; <code>null</code> if the plugin is not loaded + */ + public Long getPluginLoadTime(PluginKey pluginKey) { + return this.loadedTimestamps.get(pluginKey); + } + + /** * The initialize method that prepares the plugin container. This should get the plugin * container ready to accept plugins. * @@ -185,6 +205,7 @@ public abstract class AbstractTypeServerPluginContainer { public synchronized void loadPlugin(ServerPluginEnvironment env, boolean enabled) throws Exception { if (this.pluginManager != null) { this.pluginManager.loadPlugin(env, enabled); + this.loadedTimestamps.put(env.getPluginKey(), System.currentTimeMillis()); } else { throw new Exception("Cannot load a plugin; plugin container is not initialized yet"); } @@ -201,6 +222,7 @@ public abstract class AbstractTypeServerPluginContainer { public synchronized void unloadPlugin(PluginKey pluginKey) throws Exception { if (this.pluginManager != null) { this.pluginManager.unloadPlugin(pluginKey.getPluginName(), false); + this.loadedTimestamps.remove(pluginKey); } else { throw new Exception("Cannot unload a plugin; plugin container has been shutdown"); } @@ -218,6 +240,7 @@ public abstract class AbstractTypeServerPluginContainer { public synchronized void reloadPlugin(PluginKey pluginKey, boolean enabled) throws Exception { if (this.pluginManager != null) { this.pluginManager.reloadPlugin(pluginKey.getPluginName(), enabled); + this.loadedTimestamps.put(pluginKey, System.currentTimeMillis()); } else { throw new Exception("Cannot reload a plugin; plugin container has been shutdown"); } diff --git a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/ServerPluginsBeanTest.java b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/ServerPluginsBeanTest.java index e1738e5..935721c 100644 --- a/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/ServerPluginsBeanTest.java +++ b/modules/enterprise/server/jar/src/test/java/org/rhq/enterprise/server/plugin/ServerPluginsBeanTest.java @@ -144,6 +144,12 @@ public class ServerPluginsBeanTest extends AbstractEJB3Test { assert plugins.contains(p2) : plugins; assetLazyInitializationException(plugins.get(plugins.indexOf(p1))); assetLazyInitializationException(plugins.get(plugins.indexOf(p2))); + + long lastTimestamp; + lastTimestamp = this.serverPluginsBean.getLastConfigurationChangeTimestamp(p1.getId()); + assert lastTimestamp == getConfigurationLastModifiedTimestamp(p1); + lastTimestamp = this.serverPluginsBean.getLastConfigurationChangeTimestamp(p2.getId()); + assert lastTimestamp == getConfigurationLastModifiedTimestamp(p2); }
public void testUpdatePlugins() throws Exception { @@ -393,4 +399,22 @@ public class ServerPluginsBeanTest extends AbstractEJB3Test { getTransactionManager().rollback(); } } + + private long getConfigurationLastModifiedTimestamp(ServerPlugin plugin) { + // determine the last time the plugin config or schedule jobs changed + // return 0 value if plugin doesn't have any config whatsoever + long timestamp = 0; + + Configuration config = plugin.getPluginConfiguration(); + if (config != null) { + timestamp = config.getModifiedTime(); + } + + config = plugin.getScheduledJobsConfiguration(); + if (config != null && config.getModifiedTime() > timestamp) { + timestamp = config.getModifiedTime(); + } + + return timestamp; + } }
rhq-commits@lists.fedorahosted.org