From 28d1dbfee4256a97744ed1cebe21971adf5fdaef Mon Sep 17 00:00:00 2001 From: Justin Clark-Casey (justincc) Date: Fri, 21 Nov 2014 01:08:58 +0000 Subject: Move conditionals which control whether a task is placed in the JobEngine inside Watchdog.RunJob() (renamed from RunWhenPossible) and generalize them. --- OpenSim/Framework/Monitoring/Watchdog.cs | 39 +++++++++++++- OpenSim/Region/Framework/Scenes/ScenePresence.cs | 68 +++++++++--------------- 2 files changed, 62 insertions(+), 45 deletions(-) diff --git a/OpenSim/Framework/Monitoring/Watchdog.cs b/OpenSim/Framework/Monitoring/Watchdog.cs index ebbf095..0feec7c 100644 --- a/OpenSim/Framework/Monitoring/Watchdog.cs +++ b/OpenSim/Framework/Monitoring/Watchdog.cs @@ -454,7 +454,38 @@ namespace OpenSim.Framework.Monitoring m_watchdogTimer.Start(); } - public static void RunWhenPossible(string jobType, WaitCallback callback, string name, object obj, bool log = false) + /// + /// Run a job. + /// + /// + /// This differs from direct scheduling (e.g. Util.FireAndForget) in that a job can be run in the job + /// engine if it is running, where all jobs are currently performed in sequence on a single thread. This is + /// to prevent observed overload and server freeze problems when there are hundreds of connections which all attempt to + /// perform work at once (e.g. in conference situations). With lower numbers of connections, the small + /// delay in performing jobs in sequence rather than concurrently has not been notiecable in testing, though a future more + /// sophisticated implementation could perform jobs concurrently when the server is under low load. + /// + /// However, be advised that some callers of this function rely on all jobs being performed in sequence if any + /// jobs are performed in sequence (i.e. if jobengine is active or not). Therefore, expanding the jobengine + /// beyond a single thread will require considerable thought. + /// + /// Also, any jobs submitted must be guaranteed to complete within a reasonable timeframe (e.g. they cannot + /// incorporate a network delay with a long timeout). At the moment, work that could suffer such issues + /// should still be run directly with RunInThread(), Util.FireAndForget(), etc. This is another area where + /// the job engine could be improved and so CPU utilization improved by better management of concurrency within + /// OpenSimulator. + /// + /// General classification for the job (e.g. "RezAttachments"). + /// Callback for job. + /// Specific name of job (e.g. "RezAttachments for Joe Bloggs" + /// Object to pass to callback when run + /// If set to true then the job may be run in ths calling thread. + /// If the true then the job must never timeout. + /// If set to true then extra logging is performed. + public static void RunJob( + string jobType, WaitCallback callback, string name, object obj, + bool canRunInThisThread = false, bool mustNotTimeout = false, + bool log = false) { if (Util.FireAndForgetMethod == FireAndForgetMethod.RegressionTest) { @@ -465,8 +496,12 @@ namespace OpenSim.Framework.Monitoring if (JobEngine.IsRunning) JobEngine.QueueRequest(name, callback, obj); - else + else if (canRunInThisThread) + callback(obj); + else if (mustNotTimeout) RunInThread(callback, name, obj, log); + else + Util.FireAndForget(callback, obj, name); } } } \ No newline at end of file diff --git a/OpenSim/Region/Framework/Scenes/ScenePresence.cs b/OpenSim/Region/Framework/Scenes/ScenePresence.cs index 1d234e2..757ec43 100644 --- a/OpenSim/Region/Framework/Scenes/ScenePresence.cs +++ b/OpenSim/Region/Framework/Scenes/ScenePresence.cs @@ -1228,15 +1228,11 @@ namespace OpenSim.Region.Framework.Scenes // viewers without (e.g. v1 viewers) will not, so we still need to make this call. if (Scene.AttachmentsModule != null) { - if (Watchdog.JobEngine.IsRunning) - Watchdog.RunWhenPossible( - "RezAttachments", - o => Scene.AttachmentsModule.RezAttachments(this), - string.Format("Rez attachments for {0} in {1}", Name, Scene.Name), - null); - else - Util.FireAndForget( - o => Scene.AttachmentsModule.RezAttachments(this), null, "ScenePresence.RezAttachmentsOnLogin"); + Watchdog.RunJob( + "RezAttachments", + o => Scene.AttachmentsModule.RezAttachments(this), + string.Format("Rez attachments for {0} in {1}", Name, Scene.Name), + null); } } else @@ -1258,18 +1254,12 @@ namespace OpenSim.Region.Framework.Scenes if (attachments.Count > 0) { - if (Watchdog.JobEngine.IsRunning) - { - Watchdog.RunWhenPossible( - "StartAttachmentScripts", - o => RestartAttachmentScripts(attachments), - string.Format("Start attachment scripts for {0} in {1}", Name, Scene.Name), - null); - } - else - { - RestartAttachmentScripts(attachments); - } + Watchdog.RunJob( + "StartAttachmentScripts", + o => RestartAttachmentScripts(attachments), + string.Format("Start attachment scripts for {0} in {1}", Name, Scene.Name), + null, + true); } } @@ -1825,18 +1815,12 @@ namespace OpenSim.Region.Framework.Scenes // XXX: If we force an update after activity has completed, then multiple attachments do appear correctly on a destination region // If we do it a little bit earlier (e.g. when converting the child to a root agent) then this does not work. // This may be due to viewer code or it may be something we're not doing properly simulator side. - if (Watchdog.JobEngine.IsRunning) - { - Watchdog.RunWhenPossible( - "ScheduleAttachmentsForFullUpdate", - o => ScheduleAttachmentsForFullUpdate(), - string.Format("Schedule attachments for full update for {0} in {1}", Name, Scene.Name), - null); - } - else - { - ScheduleAttachmentsForFullUpdate(); - } + Watchdog.RunJob( + "ScheduleAttachmentsForFullUpdate", + o => ScheduleAttachmentsForFullUpdate(), + string.Format("Schedule attachments for full update for {0} in {1}", Name, Scene.Name), + null, + true); // m_log.DebugFormat( // "[SCENE PRESENCE]: Completing movement of {0} into region {1} took {2}ms", @@ -3391,7 +3375,7 @@ namespace OpenSim.Region.Framework.Scenes SentInitialDataToClient = true; // Send all scene object to the new client - Watchdog.RunWhenPossible("SendInitialDataToClient", delegate + Watchdog.RunJob("SendInitialDataToClient", delegate { // m_log.DebugFormat( // "[SCENE PRESENCE]: Sending initial data to {0} agent {1} in {2}, tp flags {3}", @@ -3409,7 +3393,7 @@ namespace OpenSim.Region.Framework.Scenes if (e != null && e is SceneObjectGroup) ((SceneObjectGroup)e).SendFullUpdateToClient(ControllingClient); } - }, string.Format("SendInitialDataToClient ({0} in {1})", Name, Scene.Name), null); + }, string.Format("SendInitialDataToClient ({0} in {1})", Name, Scene.Name),null, false, true); } /// @@ -4073,14 +4057,12 @@ namespace OpenSim.Region.Framework.Scenes // We don't need to worry about a race condition as the job to later start the scripts is also // JobEngine scheduled and so will always occur after this task. // XXX: This will not be true if JobEngine ever gets more than one thread. - if (Watchdog.JobEngine.IsRunning) - Watchdog.RunWhenPossible( - "CopyAttachments", - o => Scene.AttachmentsModule.CopyAttachments(cAgent, this), - string.Format("Copy attachments for {0} entering {1}", Name, Scene.Name), - null); - else - Scene.AttachmentsModule.CopyAttachments(cAgent, this); + Watchdog.RunJob( + "CopyAttachments", + o => Scene.AttachmentsModule.CopyAttachments(cAgent, this), + string.Format("Copy attachments for {0} entering {1}", Name, Scene.Name), + null, + true); } // This must occur after attachments are copied or scheduled to be copied, as it releases the CompleteMovement() calling thread -- cgit v1.1