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

Bugfix: configobject shutdown order #10191

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Bugfix: configobject shutdown order #10191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71c0b96
Adjust activation_priority for key config objects
w1ll-i-code Nov 15, 2024
4a39831
Update AUTHORS
w1ll-i-code Nov 12, 2024
39977d9
Activate users and notifications at the same time.
w1ll-i-code Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ vigiroux <[email protected]>
Vytenis Darulis <[email protected]>
Wenger Florian <[email protected]>
Will Frey <[email protected]>
William Calliari <[email protected]>
Winfried Angele <[email protected]>
Wolfgang Nieder <[email protected]>
XnS <[email protected]>
Expand Down
6 changes: 3 additions & 3 deletions lib/base/configobject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,10 +598,10 @@ void ConfigObject::StopObjects()
continue;

for (const ConfigObject::Ptr& object : dtype->GetObjects()) {
#ifdef I2_DEBUG
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this!

Log(LogDebug, "ConfigObject")
<< "Deactivate() called for config object '" << object->GetName() << "' with type '" << type->GetName() << "'.";
#endif /* I2_DEBUG */
<< "Deactivate() called for config object '" << object->GetName()
<< "' with type '" << type->GetName()
<< "and priority " << type->GetActivationPriority() << "'.";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<< "and priority " << type->GetActivationPriority() << "'.";
<< "' and priority '" << type->GetActivationPriority() << "'.";

object->Deactivate();
}
}
Expand Down
4 changes: 4 additions & 0 deletions lib/checker/checkercomponent.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ void CheckerComponent::Stop(bool runtimeRemoved)
{
std::unique_lock<std::mutex> lock(m_Mutex);
m_Stopped = true;
if (!runtimeRemoved) {
m_IdleCheckables.clear();
m_PendingCheckables.clear();
}
m_CV.notify_all();
}

Expand Down
2 changes: 1 addition & 1 deletion lib/db_ido_mysql/idomysqlconnection.ti
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace icinga

class IdoMysqlConnection : DbConnection
{
activation_priority 100;
activation_priority -50;

[config] String host {
default {{{ return "localhost"; }}}
Expand Down
2 changes: 1 addition & 1 deletion lib/db_ido_pgsql/idopgsqlconnection.ti
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace icinga

class IdoPgsqlConnection : DbConnection
{
activation_priority 100;
activation_priority -50;

[config] String host {
default {{{ return "localhost"; }}}
Expand Down
2 changes: 2 additions & 0 deletions lib/icinga/notification.ti
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ public:

class Notification : CustomVarObject < NotificationNameComposer
{
activation_priority -5;

Al2Klimov marked this conversation as resolved.
Show resolved Hide resolved
load_after Host;
load_after Service;

Expand Down
2 changes: 2 additions & 0 deletions lib/icinga/user.ti
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ namespace icinga

class User : CustomVarObject
{
activation_priority -5;

[config] String display_name {
get {{{
String displayName = m_DisplayName.load();
Expand Down
2 changes: 1 addition & 1 deletion lib/icingadb/icingadb.ti
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace icinga

class IcingaDB : ConfigObject
{
activation_priority 100;
activation_priority -50;
Copy link
Member

Choose a reason for hiding this comment

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

No matter whether Icinga DB purges Redis and re-inserts everything – or performs a diff – given #10151, not yet activated objects – all checkables – will effectively be deleted. This is not acceptable, so if -50 is actually needed here, we have to:

Copy link
Member

Choose a reason for hiding this comment

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

not yet activated objects – all checkables – will effectively be deleted

Idk, how you came up to that conclusion, nor how this relates to #10057 or #10151 in anyway. Icinga DB won't do anything if the checkables aren't truly deleted.

icinga2/lib/icingadb/icingadb-objects.cpp

Lines 2781 to 2782 in 3218908

} else if (!object->IsActive() &&
object->GetExtension("ConfigObjectDeleted")) { // same as in apilistener-configsync.cpp

I already checked this PR last week and noticed that Icinga DB, IDO etc. are not activated first as before, but last in the same way they are deactivated. IMHO the possible solution I am thinking of is to not touch the activation order of these objects on startup and deactivate them last on shutdown/reload, but this will not be achievable by simply changing the activation_priority field. So I wanted to wait until tomorrow and discuss it in our next meeting.


[config] String host {
default {{{ return "127.0.0.1"; }}}
Expand Down
2 changes: 1 addition & 1 deletion lib/notification/notificationcomponent.ti
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace icinga

class NotificationComponent : ConfigObject
{
activation_priority 200;
activation_priority -25;

[config] bool enable_ha (EnableHA) {
default {{{ return true; }}}
Expand Down