From 6c23481a556ea6ff35007a133f6afe850fbb8f4d Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Fri, 8 Feb 2013 23:40:28 +0100 Subject: [PATCH] Bugfix: Local events for changed attributes aren't processed at transaction commit time. Fixes #3605 --- .../replication/replicationcomponent.cpp | 10 +++-- components/replication/replicationcomponent.h | 2 +- lib/base/dynamicobject.cpp | 45 ++++++++++++------- lib/base/dynamicobject.h | 13 +++--- lib/icinga/host.cpp | 2 +- 5 files changed, 46 insertions(+), 26 deletions(-) diff --git a/components/replication/replicationcomponent.cpp b/components/replication/replicationcomponent.cpp index 5995b9cf1..3d97e035f 100644 --- 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& modifiedObjects) +void ReplicationComponent::TransactionClosingHandler(const set& modifiedObjects) { if (modifiedObjects.empty()) return; @@ -164,7 +164,9 @@ void ReplicationComponent::TransactionClosingHandler(const setGetSelf(); + 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 d3f1bd712..de965ab32 100644 --- 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& modifiedObjects); + void TransactionClosingHandler(const set& modifiedObjects); void RemoteObjectUpdateHandler(const RequestMessage& request); void RemoteObjectRemovedHandler(const RequestMessage& request); diff --git a/lib/base/dynamicobject.cpp b/lib/base/dynamicobject.cpp index 976bf6159..569ff9e70 100644 --- a/lib/base/dynamicobject.cpp +++ b/lib/base/dynamicobject.cpp @@ -22,11 +22,11 @@ using namespace icinga; double DynamicObject::m_CurrentTx = 0; -set DynamicObject::m_ModifiedObjects; +set DynamicObject::m_ModifiedObjects; boost::signal DynamicObject::OnRegistered; boost::signal DynamicObject::OnUnregistered; -boost::signal&)> DynamicObject::OnTransactionClosing; +boost::signal&)> 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::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(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 8b8172829..21e00c1d1 100644 --- 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 OnRegistered; static boost::signal OnUnregistered; - static boost::signal&)> OnTransactionClosing; + static boost::signal&)> OnTransactionClosing; ScriptTask::Ptr InvokeMethod(const String& method, const vector& 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 m_ModifiedAttributes; double m_ConfigTx; static double m_CurrentTx; - static set 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 m_ModifiedObjects; friend class DynamicType; /* for OnInitCompleted */ }; diff --git a/lib/icinga/host.cpp b/lib/icinga/host.cpp index 743bc2662..3e05f896f 100644 --- 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)