Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IcingaDB: Don't sync partially initialised objects #10151

Merged
merged 1 commit into from
Sep 18, 2024
Merged

IcingaDB: Don't sync partially initialised objects #10151

merged 1 commit into from
Sep 18, 2024

Conversation

yhabteab
Copy link
Member

@yhabteab yhabteab commented Sep 11, 2024 •
edited
Loading

This PR prevents the initial IcingaDB configuration dump from accessing partially initialised objects, which is hard to trigger deliberately, but it is possible as one of our customers is experiencing this problem. When Icinga DB attempts to dump the configuration to Redis, the objects are spliced into smaller chunks with a fixed size (500) and this PR goes through each of these chunks and excludes partially initialised objects before we end up in a nullptr dereference and crash the Icinga 2 process. Should these excluded objects later reach the activation process, they are captured via the OnActiveChanged signal and processed in IcingaDB::VersionChangedHandler() as runtime updates.

ref/IP/53428

How To Reproduce

  • First apply this patch 
    diff --git a/lib/icinga/downtime.cpp b/lib/icinga/downtime.cpp
index 98a65d417..927e53bbc 100644
--- a/lib/icinga/downtime.cpp
+++ b/lib/icinga/downtime.cpp
@@ -78,6 +78,7 @@ void Downtime::OnAllConfigLoaded()
 {
        ObjectImpl<Downtime>::OnAllConfigLoaded();

+       Utility::Sleep(60);
        if (GetServiceName().IsEmpty())
                m_Checkable = Host::GetByName(GetHostName());
        else
diff --git a/lib/icingadb/icingadb.cpp b/lib/icingadb/icingadb.cpp
index 6d5ded9cf..5948e5d3f 100644
--- a/lib/icingadb/icingadb.cpp
+++ b/lib/icingadb/icingadb.cpp
@@ -15,6 +15,7 @@
 #include "icinga/host.hpp"
 #include <boost/algorithm/string.hpp>
 #include <fstream>
+#include <iostream>
 #include <memory>
 #include <utility>

@@ -115,6 +116,15 @@ void IcingaDB::Start(bool runtimeCreated)
        });
        m_Rcon->Start();

+       RedisConnection::OnParentRedisConnected.connect([this](const RedisConnection::Ptr& parent) {
+               m_WorkQueue.Enqueue([this]() {
+                       std::cerr << "IcingaDB::OnParentRedisConnected" << std::endl;
+                       m_ConfigDumpInProgress = false;
+                       m_ConfigDumpDone = false;
+                       OnConnectedHandler();
+               });
+       });
+
        m_StatsTimer = Timer::Create();
        m_StatsTimer->SetInterval(1);
        m_StatsTimer->OnTimerExpired.connect([this](const Timer * const&) { PublishStatsTimerHandler(); });
diff --git a/lib/icingadb/redisconnection.cpp b/lib/icingadb/redisconnection.cpp
index 798a8279b..8c4ad8992 100644
--- a/lib/icingadb/redisconnection.cpp
+++ b/lib/icingadb/redisconnection.cpp
@@ -28,6 +28,8 @@
 using namespace icinga;
 namespace asio = boost::asio;

+boost::signals2::signal<void (const RedisConnection::Ptr&)> RedisConnection::OnParentRedisConnected;
+
 boost::regex RedisConnection::m_ErrAuth ("\\AERR AUTH ");

 RedisConnection::RedisConnection(const String& host, int port, const String& path, const String& password, int db,
@@ -379,6 +381,10 @@ void RedisConnection::Connect(asio::yield_context& yc)
                                callback(yc);
                        }

+                       if (m_Parent && !callback) {
+                               OnParentRedisConnected(this);
+                       }
+
                        break;
                } catch (const boost::coroutines::detail::forced_unwind&) {
                        throw;
diff --git a/lib/icingadb/redisconnection.hpp b/lib/icingadb/redisconnection.hpp
index f346ba285..72175ef5b 100644
--- a/lib/icingadb/redisconnection.hpp
+++ b/lib/icingadb/redisconnection.hpp
@@ -53,6 +53,8 @@ namespace icinga
        public:
                DECLARE_PTR_TYPEDEFS(RedisConnection);

+               static boost::signals2::signal<void (const RedisConnection::Ptr&)> OnParentRedisConnected;
+
                typedef std::vector<String> Query;
                typedef std::vector<Query> Queries;
                typedef Value Reply

  • Build and start Icinga 2 (make sure you don't already have downtimes in /var/lib/icinga2/icinga2/api)

  • Now create a downtime via the API 
    curl -k -s -S -i -u root:icinga -H 'Accept: application/json' \
 -X POST 'https://localhost:5665/v1/actions/schedule-downtime' \
 -d @<(cat <<EOF
{ "type": "Host", "filter": "host.name == \"master2\"", "start_time": $(date +%s), "end_time": $(date -v+5m +%s), "author": "icingaadmin", "comment": "IPv4 network maintenance", "pretty": true }
EOF
)

  • While the API call is blocked, restart Redis and observe

0# icinga::Application::SigAbrtHandler(int) in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
 1# _sigtramp in /usr/lib/system/libsystem_platform.dylib
 2# pthread_kill in /usr/lib/system/libsystem_pthread.dylib
 3# abort in /usr/lib/system/libsystem_c.dylib
 4# err in /usr/lib/system/libsystem_c.dylib
 5# boost::intrusive_ptr<icinga::ConfigObject>::operator->() const in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
 6# icinga::IcingaDB::GetObjectIdentifier(boost::intrusive_ptr<icinga::ConfigObject> const&) in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
 7# icinga::IcingaDB::PrepareObject(boost::intrusive_ptr<icinga::ConfigObject> const&, boost::intrusive_ptr<icinga::Dictionary>&, boost::intrusive_ptr<icinga::Dictionary>&) in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
 8# icinga::IcingaDB::CreateConfigUpdate(boost::intrusive_ptr<icinga::ConfigObject> const&, icinga::String, std::__1::map<icinga::String, std::__1::vector<icinga::String, std::__1::allocator<icinga::String>>, std::__1::less<icinga::String>, std::__1::allocator<std::__1::pair<icinga::String const, std::__1::vector<icinga::String, std::__1::allocator<icinga::String>>>>>&, std::__1::vector<boost::intrusive_ptr<icinga::Dictionary>, std::__1::allocator<boost::intrusive_ptr<icinga::Dictionary>>>&, bool) in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
 9# icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&)::operator()(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&) const in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
10# void icinga::WorkQueue::ParallelFor<std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&)>(std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>> const&, bool, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&) const&)::'lambda'()::operator()() const::'lambda'()::operator()() const in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
11# decltype(std::declval<std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>>()()) std::__1::__invoke[abi:ue170006]<void icinga::WorkQueue::ParallelFor<std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&)>(std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>> const&, bool, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&) const&)::'lambda'()::operator()() const::'lambda'()&>(std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>&&) in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
12# void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:ue170006]<void icinga::WorkQueue::ParallelFor<std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&)>(std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>> const&, bool, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&) const&)::'lambda'()::operator()() const::'lambda'()&>(void icinga::WorkQueue::ParallelFor<std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&)>(std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>> const&, bool, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&) const&)::'lambda'()::operator()() const::'lambda'()&) in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
13# std::__1::__function::__alloc_func<void icinga::WorkQueue::ParallelFor<std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&)>(std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>> const&, bool, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&) const&)::'lambda'()::operator()() const::'lambda'(), std::__1::allocator<void icinga::WorkQueue::ParallelFor<std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&)>(std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>> const&, bool, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&) const&)::'lambda'()::operator()() const::'lambda'()>, void ()>::operator()[abi:ue170006]() in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
14# std::__1::__function::__func<void icinga::WorkQueue::ParallelFor<std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&)>(std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>> const&, bool, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&) const&)::'lambda'()::operator()() const::'lambda'(), std::__1::allocator<void icinga::WorkQueue::ParallelFor<std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>>, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&)>(std::__1::vector<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>, std::__1::allocator<std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>>>> const&, bool, icinga::IcingaDB::UpdateAllConfigObjects()::$_24::operator()(boost::intrusive_ptr<icinga::Type> const&) const::'lambda'(std::__1::vector<boost::intrusive_ptr<icinga::ConfigObject>, std::__1::allocator<boost::intrusive_ptr<icinga::ConfigObject>>> const&) const&)::'lambda'()::operator()() const::'lambda'()>, void ()>::operator()() in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
15# std::__1::__function::__value_func<void ()>::operator()[abi:ue170006]() const in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
16# std::__1::function<void ()>::operator()() const in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2
17# icinga::WorkQueue::RunTaskFunction(std::__1::function<void ()> const&) in /Users/yhabteab/Workspace/icinga2/prefix/lib/icinga2/sbin/icinga2

@yhabteab yhabteab requested a review from oxzi September 11, 2024 08:31
@yhabteab yhabteab self-assigned this Sep 11, 2024
@cla-bot cla-bot bot added the cla/signed label Sep 11, 2024
@icinga-probot icinga-probot bot added area/api REST API area/configuration DSL, parser, compiler, error handling area/runtime Downtimes, comments, dependencies, events bug Something isn't working core/crash Shouldn't happen, requires attention ref/IP labels Sep 11, 2024
@yhabteab yhabteab added this to the 2.15.0 milestone Sep 11, 2024
@yhabteab
Copy link
Member Author

You can also use only this patch to reproduce it:

diff --git a/lib/icinga/downtime.cpp b/lib/icinga/downtime.cpp
index 98a65d417..927e53bbc 100644
--- a/lib/icinga/downtime.cpp
+++ b/lib/icinga/downtime.cpp
@@ -78,6 +78,7 @@ void Downtime::OnAllConfigLoaded()
 {
        ObjectImpl<Downtime>::OnAllConfigLoaded();
 
+       Utility::Sleep(60);
        if (GetServiceName().IsEmpty())
                m_Checkable = Host::GetByName(GetHostName());
        else
  • Build and start Icinga 2 (but don't start redis yet)
  • Use the API call from the PR description to create a downtime
  • And now start Redis

@julianbrost
Copy link
Contributor

My initial intuition would have been to do a check like this somewhere inside something like IcingaDB::CreateConfigUpdate(), but after having a look around the source, it's probably to have one in IcingaDB::UpdateAllConfigObjects(), otherwise, other code in there might run into a similar issue:

icinga2/lib/icingadb/icingadb-objects.cpp

Lines 285 to 302 in 8beb0b7

for (const ConfigObject::Ptr& object : chunk) {
if (lcType != GetLowerCaseTypeNameDB(object))
continue;
std::vector<Dictionary::Ptr> runtimeUpdates;
CreateConfigUpdate(object, lcType, hMSets, runtimeUpdates, false);
// Write out inital state for checkables
if (dumpState) {
String objectKey = GetObjectIdentifier(object);
Dictionary::Ptr state = SerializeState(dynamic_pointer_cast<Checkable>(object));
states.emplace_back(objectKey);
states.emplace_back(JsonEncode(state));
statesChksms.emplace_back(objectKey);
statesChksms.emplace_back(JsonEncode(new Dictionary({{"checksum", HashValue(state)}})));
}

That SerializeState(dynamic_pointer_cast<Checkable>(object)) in line 295 call probably wouldn't do too well in a similar situation where a service is created at runtime but didn't yet do the OnAllConfigLoaded().

However, the IcingaDB::ChunkObjects() function does not look like a good place for that check as it doesn't really match with what one would expect from the function name. So I'd either filter the input vector using std::remove_if before giving it to ChunkObjects() or simply perform the check when iterating over the objects.

@yhabteab yhabteab force-pushed the bugfix/do-not-sync-partially-initialised-objects branch from a13f571 to 26f43b0 Compare September 11, 2024 12:08
@julianbrost
Copy link
Contributor

Code change looks fine and like it should do the job to me. I didn't try to reproduce/test this myself so far though. So if anyone wants to step in 😅

@yhabteab yhabteab mentioned this pull request Sep 16, 2024
Open
Al2Klimov
Al2Klimov reviewed Sep 17, 2024
View reviewed changes
Copy link
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is indeed a valid fix for the specific problem described in OP.

But let me make an analogy: Your pipe is leaking (undone objects) at least at one particular point. This PR is just a bucket under that one point, whereas I offer to replace the pipe with one of stainless steel:

@yhabteab yhabteab removed the request for review from julianbrost September 18, 2024 11:14
@yhabteab yhabteab added the consider backporting Should be considered for inclusion in a bugfix release label Sep 18, 2024
@yhabteab
Copy link
Member Author

But let me make an analogy: Your pipe is leaking (undone objects) at least at one particular point. This PR is just a bucket under that one point, whereas I offer to replace the pipe with one of stainless steel:

We'll keep #10057 open for now and try to fix these kinds of bugs by just using the load_after feature without breaking DSL configs when we got the time, but for now this should fix the Icinga DB crash.

@yhabteab yhabteab removed the request for review from oxzi September 18, 2024 13:48
@yhabteab yhabteab merged commit e678f09 into master Sep 18, 2024
26 checks passed
@yhabteab yhabteab deleted the bugfix/do-not-sync-partially-initialised-objects branch September 18, 2024 13:48
@yhabteab yhabteab mentioned this pull request Sep 18, 2024
Merged
@Al2Klimov Al2Klimov mentioned this pull request Sep 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Al2Klimov Al2Klimov Al2Klimov approved these changes

Assignees

@yhabteab yhabteab

Labels
area/api REST API area/configuration DSL, parser, compiler, error handling area/runtime Downtimes, comments, dependencies, events bug Something isn't working cla/signed consider backporting Should be considered for inclusion in a bugfix release core/crash Shouldn't happen, requires attention ref/IP
Projects
None yet
Milestone
2.15.0
Development

Successfully merging this pull request may close these issues.
3 participants
@yhabteab @julianbrost @Al2Klimov