Login
[x]
Log in using an account from:
Fedora Account System
Red Hat Associate
Red Hat Customer
Or login using a Red Hat Bugzilla account
Forgot Password
Login:
Hide Forgot
Create an Account
Red Hat Bugzilla – Attachment 827463 Details for
Bug 1030063
Clean up plugin update to work synchronously
[?]
New
Simple Search
Advanced Search
My Links
Browse
Requests
Reports
Current State
Search
Tabular reports
Graphical reports
Duplicates
Other Reports
User Changes
Plotly Reports
Bug Status
Bug Severity
Non-Defaults
|
Product Dashboard
Help
Page Help!
Bug Writing Guidelines
What's new
Browser Support Policy
5.0.4.rh83 Release notes
FAQ
Guides index
User guide
Web Services
Contact
Legal
This site requires JavaScript to be enabled to function correctly, please enable it.
[patch]
Patch on c39bcd8571
0001-Bug-1030063-Plugin-update-clean-up-listener-nonsense.patch (text/plain), 19.64 KB, created by
Elias Ross
on 2013-11-21 22:07:03 UTC
(
hide
)
Description:
Patch on c39bcd8571
Filename:
MIME Type:
Creator:
Elias Ross
Created:
2013-11-21 22:07:03 UTC
Size:
19.64 KB
patch
obsolete
>From 838b7085674bc21602fe4a2f9b1427274f1cd4cb Mon Sep 17 00:00:00 2001 >From: Elias Ross <elias_ross@apple.com> >Date: Wed, 13 Nov 2013 10:29:14 -0800 >Subject: [PATCH] Bug 1030063 - Plugin update process clean up > >The plugin update is problematic as it is driven by listener ordering and >possibly has many race conditions like seen in Bug 1025844. For example, the >listener is actually added before the plugin container starts. Although >effectively, the dependent systems are initialized before be the listener is >called, this is not something predictable or clear. > >Removed the use of marker files and some unnecessary methods. >--- > .../java/org/rhq/enterprise/agent/AgentMain.java | 143 +++++++-------------- > .../org/rhq/enterprise/agent/PluginUpdate.java | 95 ++------------ > .../agent/i18n/AgentI18NResourceKeys.java | 10 +- > 3 files changed, 54 insertions(+), 194 deletions(-) > >diff --git a/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/AgentMain.java b/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/AgentMain.java >index 1e43996..0a916b7 100644 >--- a/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/AgentMain.java >+++ b/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/AgentMain.java >@@ -28,7 +28,6 @@ > import java.io.FileNotFoundException; > import java.io.FileOutputStream; > import java.io.FileWriter; >-import java.io.FilenameFilter; > import java.io.IOException; > import java.io.InputStream; > import java.io.InputStreamReader; >@@ -67,11 +66,9 @@ > import org.apache.log4j.Level; > import org.apache.log4j.LogManager; > import org.apache.log4j.xml.DOMConfigurator; >- > import org.jboss.remoting.invocation.NameBasedInvocation; > import org.jboss.remoting.security.SSLSocketBuilder; > import org.jboss.remoting.transport.http.ssl.HTTPSClientInvoker; >- > import org.rhq.core.clientapi.server.bundle.BundleServerService; > import org.rhq.core.clientapi.server.configuration.ConfigurationServerService; > import org.rhq.core.clientapi.server.content.ContentServerService; >@@ -393,6 +390,11 @@ > private boolean m_loggedNativeSystemInfoUnavailableWarning; > > /** >+ * Plugin update instance, used by management. >+ */ >+ private PluginUpdate m_pluginUpdate; >+ >+ /** > * The main method that starts the whole thing. > * > * @param args the arguments passed on the command line (e.g. java org.rhq.enterprise.agent.AgentMain arg1 arg2 arg3) >@@ -1706,11 +1708,6 @@ private boolean prepareStartupWorkRequiringServer() { > m_clientSender.addStateListener(new RegisterStateListener(), true); > } > >- // now we want to prepare to update the plugins if told to do so >- if (m_configuration.isUpdatePluginsAtStartupEnabled()) { >- updatePlugins(); >- } >- > //the next thing is to setup the conditional restart of the PC if it fails to merge > //the upgrade results with the server due to some network glitch > m_clientSender.addStateListener(new PluginContainerConditionalRestartListener(), false); >@@ -1719,12 +1716,20 @@ private boolean prepareStartupWorkRequiringServer() { > } > > /** >- * This asks that the agent update its plugins. If the RHQ Server is already up and the agent has detected it, this >- * method will immediately pull down the new/updated plugins. Otherwise, this will schedule the agent to update the >- * plugins from the server once it comes up and the agent detects it. >+ * Management method to manually update plugins. >+ * This method will fail if the server is down. >+ * @throws IllegalStateException if the container is not initialized >+ * @throws RuntimeException for any other reason (failed to download, etc.) > */ > public void updatePlugins() { >- m_clientSender.addStateListener(new UpdatePluginsStateListener(), true); >+ if (m_pluginUpdate == null) { >+ throw new IllegalStateException("plugin update uninitialized"); >+ } >+ try { >+ m_pluginUpdate.updatePlugins(); >+ } catch (Exception e) { >+ throw new RuntimeException(e); >+ } > } > > /** >@@ -1885,59 +1890,34 @@ private void rebootPluginContainer() throws Throwable { > return false; > } > >- try { >- File plugin_dir = pc_config.getPluginDirectory(); >- boolean keep_waiting = (plugin_dir.list().length == 0) >- || PluginUpdate.waitForUpdateToComplete(pc_config, 1000L); >- >- // we block until we get our plugins - there is no sense continuing until we have plugins >- // there may be instances, though, where we don't want to block (in unit tests for example) >- // so allow this to be configurable via the "update plugins at startup" flag. >- if (m_configuration.isUpdatePluginsAtStartupEnabled()) { >- boolean notified_user = false; >- >- while (keep_waiting) { >- if (!notified_user) { >- LOG.info(AgentI18NResourceKeys.WAITING_FOR_PLUGINS_WITH_DIR, plugin_dir); >- getOut().println(MSG.getMsg(AgentI18NResourceKeys.WAITING_FOR_PLUGINS)); >- notified_user = true; >- } else { >- // let's keep logging this at debug level so we don't look hung >- LOG.debug(AgentI18NResourceKeys.WAITING_FOR_PLUGINS_WITH_DIR, plugin_dir); >- } >- >- boolean updating = PluginUpdate.waitForUpdateToComplete(pc_config, 30000L); >- int after = plugin_dir.list().length; >- >- if ((after == 0) && !updating) { >- // still nothing and it doesn't look like we are downloading - try to update them again right now >- // (doing this because I saw a case where the startup update somehow happened just prior to the >- // registration finishing, so the original update was rejected by the server as "unauthorized") >- updatePluginsNow(m_clientSender); >- after = plugin_dir.list().length; >- } >+ File plugin_dir = pc_config.getPluginDirectory(); > >- keep_waiting = ((after == 0) || (updating)); >- >- if (!keep_waiting) { >- after = plugin_dir.list(new FilenameFilter() { >- public boolean accept(File dir, String name) { >- return name.endsWith(".jar"); >- } >- }).length; >- LOG.info(AgentI18NResourceKeys.DONE_WAITING_FOR_PLUGINS, after); >- getOut().println(MSG.getMsg(AgentI18NResourceKeys.DONE_WAITING_FOR_PLUGINS, after)); >- } >+ // we block until we get our plugins - there is no sense continuing until we have plugins >+ // there may be instances, though, where we don't want to block (in unit tests for example) >+ // so allow this to be configurable via the "update plugins at startup" flag. >+ m_pluginUpdate = new PluginUpdate(pc_config.getServerServices().getCoreServerService(), pc_config); >+ if (m_configuration.isUpdatePluginsAtStartupEnabled()) { >+ boolean notified_user = false; >+ // this can block forever...perhaps exit after a few tries? >+ while (true) { >+ if (!notified_user) { >+ LOG.info(AgentI18NResourceKeys.WAITING_FOR_PLUGINS_WITH_DIR, plugin_dir); >+ getOut().println(MSG.getMsg(AgentI18NResourceKeys.WAITING_FOR_PLUGINS)); >+ notified_user = true; >+ } else { >+ // let's keep logging this at debug level so we don't look hung >+ LOG.debug(AgentI18NResourceKeys.WAITING_FOR_PLUGINS_WITH_DIR, plugin_dir); >+ } >+ try { >+ m_pluginUpdate.updatePlugins(); >+ break; >+ } catch (Exception e) { >+ LOG.error(e, AgentI18NResourceKeys.UPDATING_PLUGINS_FAILURE, e); > } >- } else if (plugin_dir.list().length == 0) { >- LOG.warn(AgentI18NResourceKeys.NO_PLUGINS); >- getOut().println(MSG.getMsg(AgentI18NResourceKeys.NO_PLUGINS)); >- return false; > } >- } catch (InterruptedException e) { >- Thread.currentThread().interrupt(); >- LOG.warn(AgentI18NResourceKeys.PLUGIN_CONTAINER_INITIALIZATION_INTERRUPTED); >- getOut().println(MSG.getMsg(AgentI18NResourceKeys.PLUGIN_CONTAINER_INITIALIZATION_INTERRUPTED)); >+ } else if (plugin_dir.list().length == 0) { >+ LOG.warn(AgentI18NResourceKeys.NO_PLUGINS); >+ getOut().println(MSG.getMsg(AgentI18NResourceKeys.NO_PLUGINS)); > return false; > } > >@@ -3522,29 +3502,6 @@ private void setStarted(boolean started) { > m_startTime = (started) ? System.currentTimeMillis() : 0L; > } > >- /** >- * Immediately sends a request to the server to update the plugins. >- * >- * @param sender the sender used to comminucate with server >- * >- * @return <code>true</code> if the plugins were succesfully updated, <code>false</code> if an error occurred >- * >- * @see PluginUpdate >- */ >- private boolean updatePluginsNow(ClientCommandSender sender) { >- try { >- ClientRemotePojoFactory factory = sender.getClientRemotePojoFactory(); >- CoreServerService server = factory.getRemotePojo(CoreServerService.class); >- PluginContainerConfiguration pc_config = m_configuration.getPluginContainerConfiguration(); >- PluginUpdate plugin_update = new PluginUpdate(server, pc_config); >- plugin_update.updatePlugins(); >- return true; >- } catch (Exception e) { >- LOG.warn(e, AgentI18NResourceKeys.UPDATING_PLUGINS_FAILURE); >- return false; >- } >- } >- > private static void reconfigureJavaLogging() { > try { > LOG.debug(AgentI18NResourceKeys.RECONFIGURE_JAVA_LOGGING_START); >@@ -3582,21 +3539,6 @@ public boolean stoppedSending(ClientCommandSender sender) { > } > > /** >- * Listener that will update the plugins once the sender is able to start sending. >- */ >- private class UpdatePluginsStateListener implements ClientCommandSenderStateListener { >- public boolean startedSending(ClientCommandSender sender) { >- updatePluginsNow(sender); >- >- return false; // no need to keep listening >- } >- >- public boolean stoppedSending(ClientCommandSender sender) { >- return true; // no-op but keep listening >- } >- } >- >- /** > * Sender listener that will remove the command listener once the sender starts. It will also add the command > * listener once the sender stops. The command listener will allow us to immediately turn on the sender when the > * server sends us a message. We don't need this command listener once we know the sender has started (because that >@@ -3814,4 +3756,5 @@ public void run() { > } > } > } >+ > } >diff --git a/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/PluginUpdate.java b/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/PluginUpdate.java >index 8aa81a0..456f90f 100644 >--- a/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/PluginUpdate.java >+++ b/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/PluginUpdate.java >@@ -26,10 +26,9 @@ > import java.util.HashMap; > import java.util.List; > import java.util.Map; >+import java.util.concurrent.Semaphore; > import java.util.concurrent.TimeUnit; > import java.util.concurrent.TimeoutException; >-import java.util.concurrent.locks.ReadWriteLock; >-import java.util.concurrent.locks.ReentrantReadWriteLock; > > import mazz.i18n.Logger; > >@@ -53,59 +52,18 @@ > * @author Ian Springer > */ > public class PluginUpdate { >- private static final Logger LOG = AgentI18NFactory.getLogger(PluginUpdate.class); > >- private static final String MARKER_FILENAME = ".updatelock"; >+ private static final Logger LOG = AgentI18NFactory.getLogger(PluginUpdate.class); > > /** >- * Static lock that prohibits concurrent plugin updates. >+ * Lock that prohibits concurrent plugin updates between threads. > */ >- private static final ReadWriteLock lock = new ReentrantReadWriteLock(); >+ private static final Semaphore lock = new Semaphore(1); > > private final CoreServerService coreServerService; > private final PluginContainerConfiguration config; > > /** >- * All {@link PluginUpdate} objects know if they are currently updating plugins given a specific <code> >- * config</code>. Call this static method to ask if any plugin update object is currently updating plugins with the >- * given configuration >- * >- * @param config used to determine where the plugins are being updated >- * >- * @return <code>true</code> if a plugin updater object is currently updating plugin; <code>false</code> if all >- * plugins are up-to-date and nothing is being updated anymore. >- */ >- public static boolean isCurrentlyUpdating(PluginContainerConfiguration config) { >- File marker = new File(config.getPluginDirectory(), MARKER_FILENAME); >- return marker.exists(); >- } >- >- /** >- * Blocks the calling thread for a maximum of the given amount of milliseconds timeout waiting for a plugin update >- * to completely. This will return sooner if the update finishes early or if there is no update currently happening. >- * >- * @param config used to determine where the plugins are being updated >- * @param timeout max milliseconds to wait >- * >- * @return <code>true</code> if a plugin updater object is currently updating plugin and this method timed out; >- * <code>false</code> if all plugins are up-to-date and nothing is being updated anymore. >- * >- * @throws InterruptedException if thread was interrupted while waiting >- */ >- public static boolean waitForUpdateToComplete(PluginContainerConfiguration config, long timeout) >- throws InterruptedException { >- long time_limit = System.currentTimeMillis() + timeout; >- boolean currently_updating = true; // for us to sleep at least an initial amount before checking the first time >- >- while (currently_updating && (time_limit > System.currentTimeMillis())) { >- Thread.sleep(2000L); >- currently_updating = isCurrentlyUpdating(config); >- } >- >- return currently_updating; >- } >- >- /** > * Constructor for {@link PluginUpdate}. You can pass in a <code>null</code> <code>core_server_service</code> if you > * only plan to use this object to obtain information on the currently installed plugins and not actually update > * them. >@@ -151,13 +109,12 @@ public PluginContainerConfiguration getPluginContainerConfiguration() { > List<Plugin> updated_plugins = new ArrayList<Plugin>(); > > // block if some other thread is updating, too - we can only ever have one thread updating plugins >- if (!PluginUpdate.lock.writeLock().tryLock(3600, TimeUnit.SECONDS)) { >+ if (!PluginUpdate.lock.tryAcquire(3600, TimeUnit.SECONDS)) { > // it should never take this long to update plugins. But if it does, just barf > throw new TimeoutException(); > } > > try { >- createMarkerFile(); > > try { > List<String> disabled_plugin_names = this.config.getDisabledPlugins(); >@@ -237,15 +194,14 @@ public PluginContainerConfiguration getPluginContainerConfiguration() { > if (last_error != null) { > throw last_error; > } >- } finally { >- deleteMarkerFile(); >+ } catch (Exception e) { >+ throw e; > } > } finally { >- PluginUpdate.lock.writeLock().unlock(); >+ PluginUpdate.lock.release(); > } > > LOG.info(AgentI18NResourceKeys.UPDATING_PLUGINS_COMPLETE); >- > return updated_plugins; > } > >@@ -408,39 +364,6 @@ private void getPluginArchive(Plugin plugin_to_get) throws Exception { > return plugins; > } > >- private void createMarkerFile() { >- File marker = null; >- try { >- marker = new File(config.getPluginDirectory(), MARKER_FILENAME); >- >- // shouldn't exist, but if it does, oh well, just reuse it >- if (!marker.exists()) { >- new FileOutputStream(marker).close(); >- } >- } catch (Exception e) { >- LOG.warn(AgentI18NResourceKeys.UPDATING_PLUGINS_MARKER_CREATE_FAILURE, marker, e); >- } >- >- return; >- } >- >- private void deleteMarkerFile() { >- try { >- File marker = new File(config.getPluginDirectory(), MARKER_FILENAME); >- >- // it should exist, but if it doesn't oh well, just skip trying to delete it >- if (marker.exists()) { >- if (!marker.delete()) { >- LOG.warn(AgentI18NResourceKeys.UPDATING_PLUGINS_MARKER_DELETE_FAILURE, marker); >- } >- } >- } catch (Throwable t) { >- LOG.warn(AgentI18NResourceKeys.UPDATING_PLUGINS_MARKER_DELETE_FAILURE, MARKER_FILENAME); >- } >- >- return; >- } >- > private void deleteIllegitimatePlugins(Map<String, Plugin> current_plugins, Map<String, Plugin> latest_plugins_map) { > for (Plugin current_plugin : current_plugins.values()) { > if (!latest_plugins_map.containsKey(current_plugin.getPath())) { >@@ -473,4 +396,4 @@ private File getPluginFile(Plugin plugin) { > File file = new File(plugin_dir, plugin_filename); > return file; > } >-} >\ No newline at end of file >+} >diff --git a/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/i18n/AgentI18NResourceKeys.java b/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/i18n/AgentI18NResourceKeys.java >index 16c110a..26bc9c8 100644 >--- a/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/i18n/AgentI18NResourceKeys.java >+++ b/modules/enterprise/agent/src/main/java/org/rhq/enterprise/agent/i18n/AgentI18NResourceKeys.java >@@ -356,12 +356,6 @@ > @I18NMessage("Completed updating the plugins to their latest versions.") > String UPDATING_PLUGINS_COMPLETE = "PluginUpdate.updating-complete"; > >- @I18NMessage("Failed to create updater marker file [{0}] - will continue but agent startup may fail. If so, restart agent. Cause. {1}") >- String UPDATING_PLUGINS_MARKER_CREATE_FAILURE = "PluginUpdate.marker-create-failure"; >- >- @I18NMessage("Failed to delete updater marker file [{0}] - will continue but agent startup may fail. If so, delete the file manually.") >- String UPDATING_PLUGINS_MARKER_DELETE_FAILURE = "PluginUpdate.marker-delete-failure"; >- > @I18NMessage("The plugin [{0}] is current and does not need to be updated.") > String PLUGIN_ALREADY_AT_LATEST = "PluginUpdate.already-at-latest"; > >@@ -1608,10 +1602,10 @@ > @I18NMessage("The agent will now wait until it has registered with the server...") > String WAITING_TO_BE_REGISTERED_BEGIN = "AgentMain.waiting-to-be-registered-begin"; > >- @I18NMessage("The agent does not have plugins - it will now wait for them to be downloaded...") >+ @I18NMessage("The agent is waiting for plugins to be downloaded...") > String WAITING_FOR_PLUGINS = "AgentMain.waiting-for-plugins"; > >- @I18NMessage("The agent does not have plugins - it will now wait for them to be downloaded to [{0}]...") >+ @I18NMessage("The agent is waiting for plugins to be downloaded to [{0}]...") > String WAITING_FOR_PLUGINS_WITH_DIR = "AgentMain.waiting-for-plugins-with-dir"; > > @I18NMessage("[{0}] plugins downloaded.") >-- >1.8.1.2 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 1030063
: 827463