Bugfix: Local events for changed attributes aren't processed at transaction commit...

Fixes #3605

diff --git a/components/replication/replicationcomponent.cpp b/components/replication/replicationcomponent.cpp
index 5995b9cf1a55a01172b9896a9e772f16c85d9b51..3d97e035fcd468a9a497197b48c4b0e489a124e7 100644 (file)
--- a/components/replication/replicationcomponent.cpp
+++ b/components/replication/replicationcomponent.cpp
@@ -35,7 +35,7 @@ void ReplicationComponent::Start(void)
        DynamicObject::OnTransactionClosing.connect(boost::bind(&ReplicationComponent::TransactionClosingHandler, this, _1));
 
        Endpoint::OnConnected.connect(boost::bind(&ReplicationComponent::EndpointConnectedHandler, this, _1));
-       
+
        m_Endpoint->RegisterTopicHandler("config::ObjectUpdate",
            boost::bind(&ReplicationComponent::RemoteObjectUpdateHandler, this, _3));
        m_Endpoint->RegisterTopicHandler("config::ObjectRemoved",
@@ -155,7 +155,7 @@ void ReplicationComponent::LocalObjectUnregisteredHandler(const DynamicObject::P
            MakeObjectMessage(object, "config::ObjectRemoved", 0, false));
 }
 
-void ReplicationComponent::TransactionClosingHandler(const set<DynamicObject::Ptr>& modifiedObjects)
+void ReplicationComponent::TransactionClosingHandler(const set<DynamicObject *>& modifiedObjects)
 {
        if (modifiedObjects.empty())
                return;
@@ -164,7 +164,9 @@ void ReplicationComponent::TransactionClosingHandler(const set<DynamicObject::Pt
        msgbuf << "Sending " << modifiedObjects.size() << " replication updates.";
        Logger::Write(LogDebug, "replication", msgbuf.str());
 
-       BOOST_FOREACH(const DynamicObject::Ptr& object, modifiedObjects) {
+       BOOST_FOREACH(DynamicObject *robject, modifiedObjects) {
+               DynamicObject::Ptr object = robject->GetSelf();
+
                if (!ShouldReplicateObject(object))
                                continue;
 
@@ -204,7 +206,7 @@ void ReplicationComponent::RemoteObjectUpdateHandler(const RequestMessage& reque
                object = dtype->CreateObject(update);
 
                if (source == EndpointManager::GetInstance()->GetIdentity()) {
-                       /* the peer sent us an object that was originally created by us - 
+                       /* the peer sent us an object that was originally created by us -
                         * however it was deleted locally so we have to tell the peer to destroy
                         * its copy of the object. */
                        EndpointManager::GetInstance()->SendMulticastMessage(m_Endpoint,

diff --git a/components/replication/replicationcomponent.h b/components/replication/replicationcomponent.h
index d3f1bd712686e3da351abda1676c4955487e907f..de965ab325dd4fd55e69ad701cac01e2d7468df8 100644 (file)
--- a/components/replication/replicationcomponent.h
+++ b/components/replication/replicationcomponent.h
@@ -41,7 +41,7 @@ private:
 
        void LocalObjectRegisteredHandler(const DynamicObject::Ptr& object);
        void LocalObjectUnregisteredHandler(const DynamicObject::Ptr& object);
-       void TransactionClosingHandler(const set<DynamicObject::Ptr>& modifiedObjects);
+       void TransactionClosingHandler(const set<DynamicObject *>& modifiedObjects);
 
        void RemoteObjectUpdateHandler(const RequestMessage& request);
        void RemoteObjectRemovedHandler(const RequestMessage& request);

diff --git a/lib/base/dynamicobject.cpp b/lib/base/dynamicobject.cpp
index 976bf615985a2c32171f5f21e3ca4b4900c11d64..569ff9e70f5668fedc65fbc2da5c00d790f25938 100644 (file)
--- a/lib/base/dynamicobject.cpp
+++ b/lib/base/dynamicobject.cpp
@@ -22,11 +22,11 @@
 using namespace icinga;
 
 double DynamicObject::m_CurrentTx = 0;
-set<DynamicObject::Ptr> DynamicObject::m_ModifiedObjects;
+set<DynamicObject *> DynamicObject::m_ModifiedObjects;
 
 boost::signal<void (const DynamicObject::Ptr&)> DynamicObject::OnRegistered;
 boost::signal<void (const DynamicObject::Ptr&)> DynamicObject::OnUnregistered;
-boost::signal<void (const set<DynamicObject::Ptr>&)> DynamicObject::OnTransactionClosing;
+boost::signal<void (const set<DynamicObject *>&)> DynamicObject::OnTransactionClosing;
 
 DynamicObject::DynamicObject(const Dictionary::Ptr& serializedObject)
        : m_ConfigTx(0)
@@ -44,7 +44,22 @@ DynamicObject::DynamicObject(const Dictionary::Ptr& serializedObject)
        /* apply config state from the config item/remote update;
         * The DynamicObject::Create function takes care of restoring
         * non-config state after the object has been fully constructed */
-       InternalApplyUpdate(serializedObject, Attribute_Config, true);
+       ApplyUpdate(serializedObject, Attribute_Config);
+}
+
+DynamicObject::~DynamicObject(void)
+{
+       m_ModifiedObjects.erase(this);
+}
+
+void DynamicObject::SendLocalUpdateEvents(void)
+{
+       map<String, Value, string_iless>::iterator it;
+       for (it = m_ModifiedAttributes.begin(); it != m_ModifiedAttributes.end(); it++) {
+               OnAttributeChanged(it->first, it->second);
+       }
+
+       m_ModifiedAttributes.clear();
 }
 
 Dictionary::Ptr DynamicObject::BuildUpdate(double sinceTx, int attributeTypes) const
@@ -87,12 +102,6 @@ Dictionary::Ptr DynamicObject::BuildUpdate(double sinceTx, int attributeTypes) c
 
 void DynamicObject::ApplyUpdate(const Dictionary::Ptr& serializedUpdate,
     int allowedTypes)
-{
-       InternalApplyUpdate(serializedUpdate, allowedTypes, false);
-}
-
-void DynamicObject::InternalApplyUpdate(const Dictionary::Ptr& serializedUpdate,
-    int allowedTypes, bool suppressEvents)
 {
        double configTx = 0;
        if ((allowedTypes & Attribute_Config) != 0 &&
@@ -126,7 +135,7 @@ void DynamicObject::InternalApplyUpdate(const Dictionary::Ptr& serializedUpdate,
                if (!HasAttribute(it->first))
                        RegisterAttribute(it->first, static_cast<DynamicAttributeType>(type));
 
-               InternalSetAttribute(it->first, data, tx, suppressEvents, true);
+               InternalSetAttribute(it->first, data, tx, true);
        }
 }
 
@@ -160,7 +169,7 @@ Value DynamicObject::Get(const String& name) const
 }
 
 void DynamicObject::InternalSetAttribute(const String& name, const Value& data,
-    double tx, bool suppressEvent, bool allowEditConfig)
+    double tx, bool allowEditConfig)
 {
        DynamicAttribute attr;
        attr.Type = Attribute_Transient;
@@ -184,10 +193,12 @@ void DynamicObject::InternalSetAttribute(const String& name, const Value& data,
        if (tt.first->second.Type & Attribute_Config)
                m_ConfigTx = tx;
 
-       if (!suppressEvent) {
-               m_ModifiedObjects.insert(GetSelf());
-               OnAttributeChanged(name, oldValue);
-       }
+       m_ModifiedObjects.insert(this);
+
+       /* Use insert() rather than [] so we don't overwrite
+        * an existing oldValue if the attribute was previously
+        * changed in the same transaction */
+       m_ModifiedAttributes.insert(make_pair(name, oldValue));
 }
 
 Value DynamicObject::InternalGetAttribute(const String& name) const
@@ -454,6 +465,10 @@ void DynamicObject::BeginTx(void)
 
 void DynamicObject::FinishTx(void)
 {
+       BOOST_FOREACH(DynamicObject *object, m_ModifiedObjects) {
+               object->SendLocalUpdateEvents();
+       }
+
        OnTransactionClosing(m_ModifiedObjects);
        m_ModifiedObjects.clear();
 

diff --git a/lib/base/dynamicobject.h b/lib/base/dynamicobject.h
index 8b8172829e4bed9efe898074238898ff9142521f..21e00c1d13585b1a248dae958db05a9b2041b97a 100644 (file)
--- a/lib/base/dynamicobject.h
+++ b/lib/base/dynamicobject.h
@@ -79,6 +79,7 @@ public:
        typedef AttributeMap::const_iterator AttributeConstIterator;
 
        DynamicObject(const Dictionary::Ptr& serializedObject);
+       ~DynamicObject(void);
 
        Dictionary::Ptr BuildUpdate(double sinceTx, int attributeTypes) const;
        void ApplyUpdate(const Dictionary::Ptr& serializedUpdate, int allowedTypes);
@@ -95,7 +96,7 @@ public:
 
        static boost::signal<void (const DynamicObject::Ptr&)> OnRegistered;
        static boost::signal<void (const DynamicObject::Ptr&)> OnUnregistered;
-       static boost::signal<void (const set<DynamicObject::Ptr>&)> OnTransactionClosing;
+       static boost::signal<void (const set<DynamicObject *>&)> OnTransactionClosing;
 
        ScriptTask::Ptr InvokeMethod(const String& method,
            const vector<Value>& arguments, ScriptTask::CompletionCallback callback);
@@ -135,17 +136,19 @@ protected:
        virtual void OnInitCompleted(void);
 
 private:
-       void InternalSetAttribute(const String& name, const Value& data, double tx, bool suppressEvent = false, bool allowEditConfig = false);
+       void InternalSetAttribute(const String& name, const Value& data, double tx, bool allowEditConfig = false);
        Value InternalGetAttribute(const String& name) const;
+       void SendLocalUpdateEvents(void);
 
        AttributeMap m_Attributes;
+       map<String, Value, string_iless> m_ModifiedAttributes;
        double m_ConfigTx;
 
        static double m_CurrentTx;
 
-       static set<DynamicObject::Ptr> m_ModifiedObjects;
-
-       void InternalApplyUpdate(const Dictionary::Ptr& serializedUpdate, int allowedTypes, bool suppressEvents);
+       /* This has to be a set of raw pointers because the DynamicObject
+        * constructor has to be able to insert objects into this list. */
+       static set<DynamicObject *> m_ModifiedObjects;
 
        friend class DynamicType; /* for OnInitCompleted */
 };

diff --git a/lib/icinga/host.cpp b/lib/icinga/host.cpp
index 743bc266272fae894fa7d665a9265eafc08ababb..3e05f896fd20dc1df13809690074f03e054104c8 100644 (file)
--- a/lib/icinga/host.cpp
+++ b/lib/icinga/host.cpp
@@ -45,7 +45,7 @@ void Host::OnInitCompleted(void)
        HostGroup::InvalidateMembersCache();
        DowntimeProcessor::InvalidateDowntimeCache();
 
-       Event::Post(boost::bind(&Host::UpdateSlaveServices, this));
+       UpdateSlaveServices();
 }
 
 Host::~Host(void)