Changeset 3eb035

Timestamp:

Jul 2, 2012, 7:54:13 AM (13 years ago)

Author:

Frederik Heber <heber@…>

Branches:

Action_Thermostats, Add_AtomRandomPerturbation, Add_FitFragmentPartialChargesAction, Add_RotateAroundBondAction, Add_SelectAtomByNameAction, Added_ParseSaveFragmentResults, AddingActions_SaveParseParticleParameters, Adding_Graph_to_ChangeBondActions, Adding_MD_integration_tests, Adding_ParticleName_to_Atom, Adding_StructOpt_integration_tests, AtomFragments, Automaking_mpqc_open, AutomationFragmentation_failures, Candidate_v1.5.4, Candidate_v1.6.0, Candidate_v1.6.1, ChangeBugEmailaddress, ChangingTestPorts, ChemicalSpaceEvaluator, CombiningParticlePotentialParsing, Combining_Subpackages, Debian_Package_split, Debian_package_split_molecuildergui_only, Disabling_MemDebug, Docu_Python_wait, EmpiricalPotential_contain_HomologyGraph, EmpiricalPotential_contain_HomologyGraph_documentation, Enable_parallel_make_install, Enhance_userguide, Enhanced_StructuralOptimization, Enhanced_StructuralOptimization_continued, Example_ManyWaysToTranslateAtom, Exclude_Hydrogens_annealWithBondGraph, FitPartialCharges_GlobalError, Fix_BoundInBox_CenterInBox_MoleculeActions, Fix_ChargeSampling_PBC, Fix_ChronosMutex, Fix_FitPartialCharges, Fix_FitPotential_needs_atomicnumbers, Fix_ForceAnnealing, Fix_IndependentFragmentGrids, Fix_ParseParticles, Fix_ParseParticles_split_forward_backward_Actions, Fix_PopActions, Fix_QtFragmentList_sorted_selection, Fix_Restrictedkeyset_FragmentMolecule, Fix_StatusMsg, Fix_StepWorldTime_single_argument, Fix_Verbose_Codepatterns, Fix_fitting_potentials, Fixes, ForceAnnealing_goodresults, ForceAnnealing_oldresults, ForceAnnealing_tocheck, ForceAnnealing_with_BondGraph, ForceAnnealing_with_BondGraph_continued, ForceAnnealing_with_BondGraph_continued_betteresults, ForceAnnealing_with_BondGraph_contraction-expansion, FragmentAction_writes_AtomFragments, FragmentMolecule_checks_bonddegrees, GeometryObjects, Gui_Fixes, Gui_displays_atomic_force_velocity, ImplicitCharges, IndependentFragmentGrids, IndependentFragmentGrids_IndividualZeroInstances, IndependentFragmentGrids_IntegrationTest, IndependentFragmentGrids_Sole_NN_Calculation, JobMarket_RobustOnKillsSegFaults, JobMarket_StableWorkerPool, JobMarket_unresolvable_hostname_fix, MoreRobust_FragmentAutomation, ODR_violation_mpqc_open, PartialCharges_OrthogonalSummation, PdbParser_setsAtomName, PythonUI_with_named_parameters, QtGui_reactivate_TimeChanged_changes, Recreated_GuiChecks, Rewrite_FitPartialCharges, RotateToPrincipalAxisSystem_UndoRedo, SaturateAtoms_findBestMatching, SaturateAtoms_singleDegree, StoppableMakroAction, Subpackage_CodePatterns, Subpackage_JobMarket, Subpackage_LinearAlgebra, Subpackage_levmar, Subpackage_mpqc_open, Subpackage_vmg, Switchable_LogView, ThirdParty_MPQC_rebuilt_buildsystem, TrajectoryDependenant_MaxOrder, TremoloParser_IncreasedPrecision, TremoloParser_MultipleTimesteps, TremoloParser_setsAtomName, Ubuntu_1604_changes, stable

Children:

86a1e8

Parents:

d76161

git-author:

Frederik Heber <heber@…> (04/24/12 10:38:54)

git-committer:

Frederik Heber <heber@…> (07/02/12 07:54:13)

Message:

FIX: Removed channel WorkerRemoved in WorkerPool as it may cause cyclic updates in FragmentScheduler.

• If jobs are already present in the queue and an idle worker arrices, then marking a worker busy will cause an update inside the callback that is used to send the job to the new idle worker. This will trigger the same notification that is not yet removed to be called in notifyAll(). This causes a cycle in the updates.

Location:

src/Fragmentation/Automation

Files:

• Pool/WorkerPool.cpp (modified) (2 diffs)
• Pool/WorkerPool.hpp (modified) (1 diff)
• unittests/WorkerPoolUnitTest.cpp (modified) (8 diffs)
• unittests/WorkerPoolUnitTest.hpp (modified) (1 diff)

src/Fragmentation/Automation/Pool/WorkerPool.cpp

@@ -154 +154 @@
   Pool_t::iterator iter = pool.find( address );
   if (iter != pool.end()) {
-    OBSERVE;
-    NOTIFY(WorkerRemoved);
     Idle_Queue_t::iterator idleiter = getIdleWorker(address);
     if (idleiter != idle_queue.end())
@@ -182 +180 @@
 void WorkerPool::removeAllWorkers()
 {
-  OBSERVE;
-  NOTIFY(WorkerRemoved);
   // empty pool and queue
   idle_queue.clear();

src/Fragmentation/Automation/Pool/WorkerPool.hpp

@@ -39 +39 @@
   ~WorkerPool();
 
+  /** Channels for this observable.
+   *
+   * \note Be especially cautious of cyclic updates here as the updates are
+   *       used by \ref FragmentScheduler to send new jobs to idle workers!
+   *       If e.g. WorkerRemoved is added, then the marking of the one idle
+   *       worker as now busy inside the callback will cause a cycle in the
+   *       update, as the notification for WorkerIdle/Added is not yet
+   *       removed because we are still inside the callback!
+   */
   enum NotificationType {
     WorkerIdle,
     WorkerAdded,
-    WorkerRemoved,
     NotificationType_MAX  // denotes the maximum of available notification types
   };

src/Fragmentation/Automation/unittests/WorkerPoolUnitTest.cpp

@@ -52 +52 @@
   pool = new WorkerPool();
   addobserver = new NotificationObserver(pool->getChannel(WorkerPool::WorkerAdded));
-  removeobserver = new NotificationObserver(pool->getChannel(WorkerPool::WorkerRemoved));
   idleobserver = new NotificationObserver(pool->getChannel(WorkerPool::WorkerIdle));
 
   // and sign on
   pool->signOn(addobserver, WorkerPool::WorkerAdded);
-  pool->signOn(removeobserver, WorkerPool::WorkerRemoved);
   pool->signOn(idleobserver, WorkerPool::WorkerIdle);
 }
@@ -64 +62 @@
 {
   pool->signOff(addobserver, WorkerPool::WorkerAdded);
-  pool->signOff(removeobserver, WorkerPool::WorkerRemoved);
   pool->signOff(idleobserver, WorkerPool::WorkerIdle);
 
-  delete removeobserver;
   delete addobserver;
   delete idleobserver;
@@ -92 +88 @@
   CPPUNIT_ASSERT_EQUAL( address, pool->begin_idle()->second );
   CPPUNIT_ASSERT( addobserver->wasNotified );
-  CPPUNIT_ASSERT( !removeobserver->wasNotified );
   CPPUNIT_ASSERT( idleobserver->wasNotified );
   addobserver->wasNotified = false;
@@ -101 +96 @@
   CPPUNIT_ASSERT( !pool->removeWorker(otheraddress) );
   CPPUNIT_ASSERT( !addobserver->wasNotified );
-  CPPUNIT_ASSERT( !removeobserver->wasNotified );
   CPPUNIT_ASSERT( !idleobserver->wasNotified );
 
@@ -114 +108 @@
   CPPUNIT_ASSERT_EQUAL( (size_t)0, pool->pool.size() );
   CPPUNIT_ASSERT( !addobserver->wasNotified );
-  CPPUNIT_ASSERT( removeobserver->wasNotified );
   CPPUNIT_ASSERT( !idleobserver->wasNotified );
-  removeobserver->wasNotified = false;
 }
 
@@ -133 +125 @@
   CPPUNIT_ASSERT_EQUAL( (size_t)1, pool->pool.size() );
   CPPUNIT_ASSERT( addobserver->wasNotified );
-  CPPUNIT_ASSERT( !removeobserver->wasNotified );
   CPPUNIT_ASSERT( idleobserver->wasNotified );
   addobserver->wasNotified = false;
@@ -163 +154 @@
   CPPUNIT_ASSERT_EQUAL( (size_t)1, pool->busy_queue.size() );
   CPPUNIT_ASSERT( !addobserver->wasNotified );
-  CPPUNIT_ASSERT( !removeobserver->wasNotified );
   CPPUNIT_ASSERT( !idleobserver->wasNotified );
 
@@ -172 +162 @@
   CPPUNIT_ASSERT_EQUAL( (size_t)0, pool->busy_queue.size() );
   CPPUNIT_ASSERT( !addobserver->wasNotified );
-  CPPUNIT_ASSERT( !removeobserver->wasNotified );
   CPPUNIT_ASSERT( idleobserver->wasNotified );
 }

src/Fragmentation/Automation/unittests/WorkerPoolUnitTest.hpp

@@ -38 +38 @@
       WorkerPool *pool;
       NotificationObserver *addobserver;
-      NotificationObserver *removeobserver;
       NotificationObserver *idleobserver;
 };